From 8d99366f09fe4a52971fe223aeff255c2d6cf08d Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 25 Sep 2024 11:49:35 +0300 Subject: [PATCH 01/18] Sketch fast VM integration in oneshot executor --- core/lib/vm_executor/src/oneshot/mod.rs | 180 ++++++++++++++++++------ 1 file changed, 134 insertions(+), 46 deletions(-) diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs index cb75f396b5d5..a5dee46df6ee 100644 --- a/core/lib/vm_executor/src/oneshot/mod.rs +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -19,20 +19,21 @@ use zksync_multivm::{ executor::{OneshotExecutor, TransactionValidator}, storage::{ReadStorage, StoragePtr, StorageView, WriteStorage}, tracer::{ValidationError, ValidationParams}, - ExecutionResult, OneshotEnv, OneshotTracingParams, OneshotTransactionExecutionResult, + Call, ExecutionResult, OneshotEnv, OneshotTracingParams, OneshotTransactionExecutionResult, StoredL2BlockEnv, TxExecutionArgs, TxExecutionMode, VmExecutionMode, VmInterface, }, - tracers::{CallTracer, StorageInvocations, ValidationTracer}, + tracers::{CallTracer, StorageInvocations, TracerDispatcher, ValidationTracer}, utils::adjust_pubdata_price_for_tx, - vm_latest::HistoryDisabled, + vm_latest::{HistoryDisabled, HistoryEnabled}, zk_evm_latest::ethereum_types::U256, - LegacyVmInstance, MultiVMTracer, + FastVmInstance, HistoryMode, LegacyVmInstance, MultiVMTracer, }; use zksync_types::{ block::pack_block_info, get_nonce_key, l2::L2Tx, utils::{decompose_full_nonce, nonces_to_full_nonce, storage_key_for_eth_balance}, + vm::FastVmMode, AccountTreeId, Nonce, StorageKey, Transaction, SYSTEM_CONTEXT_ADDRESS, SYSTEM_CONTEXT_CURRENT_L2_BLOCK_INFO_POSITION, SYSTEM_CONTEXT_CURRENT_TX_ROLLING_HASH_POSITION, }; @@ -53,6 +54,7 @@ mod mock; /// Main [`OneshotExecutor`] implementation used by the API server. #[derive(Debug, Default)] pub struct MainOneshotExecutor { + fast_vm_mode: FastVmMode, missed_storage_invocation_limit: usize, execution_latency_histogram: Option<&'static vise::Histogram>, } @@ -62,11 +64,22 @@ impl MainOneshotExecutor { /// The limit is applied for calls and gas estimations, but not during transaction validation. pub fn new(missed_storage_invocation_limit: usize) -> Self { Self { + fast_vm_mode: FastVmMode::Old, missed_storage_invocation_limit, execution_latency_histogram: None, } } + /// Sets the fast VM mode used by this executor. + pub fn set_fast_vm_mode(&mut self, fast_vm_mode: FastVmMode) { + if !matches!(fast_vm_mode, FastVmMode::Old) { + tracing::warn!( + "Running new VM with mode {fast_vm_mode:?}; this can lead to incorrect node behavior" + ); + } + self.fast_vm_mode = fast_vm_mode; + } + /// Sets a histogram for measuring VM execution latency. pub fn set_execution_latency_histogram( &mut self, @@ -96,34 +109,28 @@ where } }; let execution_latency_histogram = self.execution_latency_histogram; + let fast_vm_mode = if params.trace_calls { + FastVmMode::Old // the fast VM doesn't support call tracing + } else { + self.fast_vm_mode + }; tokio::task::spawn_blocking(move || { - let mut tracers = vec![]; - let mut calls_result = Arc::>::default(); - if params.trace_calls { - tracers.push(CallTracer::new(calls_result.clone()).into_tracer_pointer()); - } - tracers.push( - StorageInvocations::new(missed_storage_invocation_limit).into_tracer_pointer(), + let executor = VmSandbox::new( + fast_vm_mode, + storage, + env, + args, + execution_latency_histogram, ); - - let executor = VmSandbox::new(storage, env, args, execution_latency_histogram); - let mut result = executor.apply(|vm, transaction| { - let (compression_result, tx_result) = vm - .inspect_transaction_with_bytecode_compression( - &mut tracers.into(), - transaction, - true, - ); - OneshotTransactionExecutionResult { - tx_result: Box::new(tx_result), - compression_result: compression_result.map(drop), - call_traces: vec![], - } - }); - - result.call_traces = Arc::make_mut(&mut calls_result).take().unwrap_or_default(); - result + executor.apply(|vm, transaction| { + vm.inspect_transaction_with_bytecode_compression( + missed_storage_invocation_limit, + params, + transaction, + true, + ) + }) }) .await .context("VM execution panicked") @@ -158,12 +165,16 @@ where let tracers = vec![validation_tracer.into_tracer_pointer()]; let executor = VmSandbox::new( + FastVmMode::Old, storage, env, TxExecutionArgs::for_validation(tx), execution_latency_histogram, ); let exec_result = executor.apply(|vm, transaction| { + let Vm::Legacy(vm) = vm else { + unreachable!("Fast VM is never used for validation yet"); + }; vm.push_transaction(transaction); vm.inspect(&mut tracers.into(), VmExecutionMode::OneTx) }); @@ -182,9 +193,71 @@ where } } +#[derive(Debug)] +enum Vm { + Legacy(LegacyVmInstance), + Fast(FastVmInstance), +} + +impl Vm { + fn inspect_transaction_with_bytecode_compression( + &mut self, + missed_storage_invocation_limit: usize, + params: OneshotTracingParams, + tx: Transaction, + with_compression: bool, + ) -> OneshotTransactionExecutionResult { + let mut calls_result = Arc::>::default(); + let (compression_result, tx_result) = match self { + Self::Legacy(vm) => { + let mut tracers = Self::create_legacy_tracers( + missed_storage_invocation_limit, + params.trace_calls.then(|| calls_result.clone()), + ); + vm.inspect_transaction_with_bytecode_compression(&mut tracers, tx, with_compression) + } + Self::Fast(vm) => { + assert!( + !params.trace_calls, + "Call tracing is not supported by fast VM yet" + ); + let legacy_tracers = Self::create_legacy_tracers::( + missed_storage_invocation_limit, + None, + ); + let mut full_tracer = (legacy_tracers.into(), ()); + vm.inspect_transaction_with_bytecode_compression( + &mut full_tracer, + tx, + with_compression, + ) + } + }; + + OneshotTransactionExecutionResult { + tx_result: Box::new(tx_result), + compression_result: compression_result.map(drop), + call_traces: Arc::make_mut(&mut calls_result).take().unwrap_or_default(), + } + } + + fn create_legacy_tracers( + missed_storage_invocation_limit: usize, + calls_result: Option>>>, + ) -> TracerDispatcher, H> { + let mut tracers = vec![]; + if let Some(calls_result) = calls_result { + tracers.push(CallTracer::new(calls_result).into_tracer_pointer()); + } + tracers + .push(StorageInvocations::new(missed_storage_invocation_limit).into_tracer_pointer()); + tracers.into() + } +} + #[derive(Debug)] struct VmSandbox { - vm: Box>, + vm: Vm, storage_view: StoragePtr>, transaction: Transaction, execution_latency_histogram: Option<&'static vise::Histogram>, @@ -193,6 +266,7 @@ struct VmSandbox { impl VmSandbox { /// This method is blocking. fn new( + fast_vm_mode: FastVmMode, storage: S, mut env: OneshotEnv, execution_args: TxExecutionArgs, @@ -212,13 +286,24 @@ impl VmSandbox { }; let storage_view = storage_view.to_rc_ptr(); - let vm = Box::new(LegacyVmInstance::new_with_specific_version( - env.l1_batch, - env.system, - storage_view.clone(), - protocol_version.into_api_vm_version(), - )); - + let vm = match fast_vm_mode { + FastVmMode::Old => Vm::Legacy(LegacyVmInstance::new_with_specific_version( + env.l1_batch, + env.system, + storage_view.clone(), + protocol_version.into_api_vm_version(), + )), + FastVmMode::New => Vm::Fast(FastVmInstance::fast( + env.l1_batch, + env.system, + storage_view.clone(), + )), + FastVmMode::Shadow => Vm::Fast(FastVmInstance::shadowed( + env.l1_batch, + env.system, + storage_view.clone(), + )), + }; Self { vm, storage_view, @@ -277,7 +362,7 @@ impl VmSandbox { pub(super) fn apply(mut self, apply_fn: F) -> T where - F: FnOnce(&mut LegacyVmInstance, Transaction) -> T, + F: FnOnce(&mut Vm, Transaction) -> T, { let tx_id = format!( "{:?}-{}", @@ -286,19 +371,22 @@ impl VmSandbox { ); let started_at = Instant::now(); - let result = apply_fn(&mut *self.vm, self.transaction); + let result = apply_fn(&mut self.vm, self.transaction); let vm_execution_took = started_at.elapsed(); if let Some(histogram) = self.execution_latency_histogram { histogram.observe(vm_execution_took); } - let memory_metrics = self.vm.record_vm_memory_metrics(); - metrics::report_vm_memory_metrics( - &tx_id, - &memory_metrics, - vm_execution_took, - &self.storage_view.borrow().stats(), - ); + if let Vm::Legacy(vm) = &self.vm { + let memory_metrics = vm.record_vm_memory_metrics(); + metrics::report_vm_memory_metrics( + &tx_id, + &memory_metrics, + vm_execution_took, + &self.storage_view.borrow().stats(), + ); + // FIXME: Memory metrics don't work for the fast VM yet + } result } } From 1d81e8826dfbe05c86117ad1da72438289332658 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 25 Sep 2024 15:32:16 +0300 Subject: [PATCH 02/18] Move `StorageWithOverrides` to VM interface --- core/lib/vm_interface/src/storage/mod.rs | 2 + .../lib/vm_interface/src/storage/overrides.rs | 70 +++++++++ .../src/execution_sandbox/storage.rs | 148 ++++++------------ 3 files changed, 116 insertions(+), 104 deletions(-) create mode 100644 core/lib/vm_interface/src/storage/overrides.rs diff --git a/core/lib/vm_interface/src/storage/mod.rs b/core/lib/vm_interface/src/storage/mod.rs index 6cdcd33db682..aade56ca5d96 100644 --- a/core/lib/vm_interface/src/storage/mod.rs +++ b/core/lib/vm_interface/src/storage/mod.rs @@ -5,11 +5,13 @@ use zksync_types::{get_known_code_key, StorageKey, StorageValue, H256}; pub use self::{ // Note, that `test_infra` of the bootloader tests relies on this value to be exposed in_memory::{InMemoryStorage, IN_MEMORY_STORAGE_DEFAULT_NETWORK_ID}, + overrides::StorageWithOverrides, snapshot::{StorageSnapshot, StorageWithSnapshot}, view::{ImmutableStorageView, StorageView, StorageViewCache, StorageViewStats}, }; mod in_memory; +mod overrides; mod snapshot; mod view; diff --git a/core/lib/vm_interface/src/storage/overrides.rs b/core/lib/vm_interface/src/storage/overrides.rs new file mode 100644 index 000000000000..ad5a3d8624f1 --- /dev/null +++ b/core/lib/vm_interface/src/storage/overrides.rs @@ -0,0 +1,70 @@ +//! VM storage functionality specifically used in the VM sandbox. + +use std::{ + collections::{HashMap, HashSet}, + fmt, +}; + +use zksync_types::{AccountTreeId, StorageKey, StorageValue, H256}; + +use super::ReadStorage; + +/// A storage view that allows to override some of the storage values. +#[derive(Debug)] +pub struct StorageWithOverrides { + storage_handle: S, + overridden_slots: HashMap, + overridden_factory_deps: HashMap>, + empty_accounts: HashSet, +} + +impl StorageWithOverrides { + /// Creates a new storage view based on the underlying storage. + pub fn new(storage: S) -> Self { + Self { + storage_handle: storage, + overridden_slots: HashMap::new(), + overridden_factory_deps: HashMap::new(), + empty_accounts: HashSet::new(), + } + } + + pub fn set_value(&mut self, key: StorageKey, value: StorageValue) { + self.overridden_slots.insert(key, value); + } + + pub fn store_factory_dep(&mut self, hash: H256, code: Vec) { + self.overridden_factory_deps.insert(hash, code); + } + + pub fn insert_erased_account(&mut self, account: AccountTreeId) { + self.empty_accounts.insert(account); + } +} + +impl ReadStorage for StorageWithOverrides { + fn read_value(&mut self, key: &StorageKey) -> StorageValue { + if let Some(value) = self.overridden_slots.get(key) { + return *value; + } + if self.empty_accounts.contains(key.account()) { + return H256::zero(); + } + self.storage_handle.read_value(key) + } + + fn is_write_initial(&mut self, key: &StorageKey) -> bool { + self.storage_handle.is_write_initial(key) + } + + fn load_factory_dep(&mut self, hash: H256) -> Option> { + self.overridden_factory_deps + .get(&hash) + .cloned() + .or_else(|| self.storage_handle.load_factory_dep(hash)) + } + + fn get_enumeration_index(&mut self, key: &StorageKey) -> Option { + self.storage_handle.get_enumeration_index(key) + } +} diff --git a/core/node/api_server/src/execution_sandbox/storage.rs b/core/node/api_server/src/execution_sandbox/storage.rs index bf775d484906..c80356f6e36e 100644 --- a/core/node/api_server/src/execution_sandbox/storage.rs +++ b/core/node/api_server/src/execution_sandbox/storage.rs @@ -1,127 +1,67 @@ //! VM storage functionality specifically used in the VM sandbox. -use std::{ - collections::{HashMap, HashSet}, - fmt, -}; - -use zksync_multivm::interface::storage::ReadStorage; +use zksync_multivm::interface::storage::{ReadStorage, StorageWithOverrides}; use zksync_types::{ api::state_override::{OverrideState, StateOverride}, get_code_key, get_known_code_key, get_nonce_key, utils::{decompose_full_nonce, nonces_to_full_nonce, storage_key_for_eth_balance}, - AccountTreeId, StorageKey, StorageValue, H256, + AccountTreeId, StorageKey, H256, }; use zksync_utils::{h256_to_u256, u256_to_h256}; -/// A storage view that allows to override some of the storage values. -#[derive(Debug)] -pub(super) struct StorageWithOverrides { - storage_handle: S, - overridden_slots: HashMap, - overridden_factory_deps: HashMap>, - overridden_accounts: HashSet, -} - -impl StorageWithOverrides { - /// Creates a new storage view based on the underlying storage. - pub(super) fn new(storage: S, state_override: &StateOverride) -> Self { - let mut this = Self { - storage_handle: storage, - overridden_slots: HashMap::new(), - overridden_factory_deps: HashMap::new(), - overridden_accounts: HashSet::new(), - }; - this.apply_state_override(state_override); - this - } - - fn apply_state_override(&mut self, state_override: &StateOverride) { - for (account, overrides) in state_override.iter() { - if let Some(balance) = overrides.balance { - let balance_key = storage_key_for_eth_balance(account); - self.overridden_slots - .insert(balance_key, u256_to_h256(balance)); - } +/// This method is blocking. +pub(super) fn apply_state_override( + storage: S, + state_override: &StateOverride, +) -> StorageWithOverrides { + let mut storage = StorageWithOverrides::new(storage); + for (account, overrides) in state_override.iter() { + if let Some(balance) = overrides.balance { + let balance_key = storage_key_for_eth_balance(account); + storage.set_value(balance_key, u256_to_h256(balance)); + } - if let Some(nonce) = overrides.nonce { - let nonce_key = get_nonce_key(account); - let full_nonce = self.read_value(&nonce_key); - let (_, deployment_nonce) = decompose_full_nonce(h256_to_u256(full_nonce)); - let new_full_nonce = u256_to_h256(nonces_to_full_nonce(nonce, deployment_nonce)); - self.overridden_slots.insert(nonce_key, new_full_nonce); - } + if let Some(nonce) = overrides.nonce { + let nonce_key = get_nonce_key(account); + let full_nonce = storage.read_value(&nonce_key); + let (_, deployment_nonce) = decompose_full_nonce(h256_to_u256(full_nonce)); + let new_full_nonce = u256_to_h256(nonces_to_full_nonce(nonce, deployment_nonce)); + storage.set_value(nonce_key, new_full_nonce); + } - if let Some(code) = &overrides.code { - let code_key = get_code_key(account); - let code_hash = code.hash(); - self.overridden_slots.insert(code_key, code_hash); - let known_code_key = get_known_code_key(&code_hash); - self.overridden_slots - .insert(known_code_key, H256::from_low_u64_be(1)); - self.store_factory_dep(code_hash, code.clone().into_bytes()); - } + if let Some(code) = &overrides.code { + let code_key = get_code_key(account); + let code_hash = code.hash(); + storage.set_value(code_key, code_hash); + let known_code_key = get_known_code_key(&code_hash); + storage.set_value(known_code_key, H256::from_low_u64_be(1)); + storage.store_factory_dep(code_hash, code.clone().into_bytes()); + } - match &overrides.state { - Some(OverrideState::State(state)) => { - let account = AccountTreeId::new(*account); - self.override_account_state_diff(account, state); - self.overridden_accounts.insert(account); + match &overrides.state { + Some(OverrideState::State(state)) => { + let account = AccountTreeId::new(*account); + for (&key, &value) in state { + storage.set_value(StorageKey::new(account, key), value); } - Some(OverrideState::StateDiff(state_diff)) => { - let account = AccountTreeId::new(*account); - self.override_account_state_diff(account, state_diff); + storage.insert_erased_account(account); + } + Some(OverrideState::StateDiff(state_diff)) => { + let account = AccountTreeId::new(*account); + for (&key, &value) in state_diff { + storage.set_value(StorageKey::new(account, key), value); } - None => { /* do nothing */ } } + None => { /* do nothing */ } } } - - fn store_factory_dep(&mut self, hash: H256, code: Vec) { - self.overridden_factory_deps.insert(hash, code); - } - - fn override_account_state_diff( - &mut self, - account: AccountTreeId, - state_diff: &HashMap, - ) { - let account_slots = state_diff - .iter() - .map(|(&slot, &value)| (StorageKey::new(account, slot), value)); - self.overridden_slots.extend(account_slots); - } -} - -impl ReadStorage for StorageWithOverrides { - fn read_value(&mut self, key: &StorageKey) -> StorageValue { - if let Some(value) = self.overridden_slots.get(key) { - return *value; - } - if self.overridden_accounts.contains(key.account()) { - return H256::zero(); - } - self.storage_handle.read_value(key) - } - - fn is_write_initial(&mut self, key: &StorageKey) -> bool { - self.storage_handle.is_write_initial(key) - } - - fn load_factory_dep(&mut self, hash: H256) -> Option> { - self.overridden_factory_deps - .get(&hash) - .cloned() - .or_else(|| self.storage_handle.load_factory_dep(hash)) - } - - fn get_enumeration_index(&mut self, key: &StorageKey) -> Option { - self.storage_handle.get_enumeration_index(key) - } + storage } #[cfg(test)] mod tests { + use std::collections::HashMap; + use zksync_multivm::interface::storage::InMemoryStorage; use zksync_types::{ api::state_override::{Bytecode, OverrideAccount}, @@ -184,7 +124,7 @@ mod tests { storage.set_value(retained_key, H256::repeat_byte(0xfe)); let erased_key = StorageKey::new(AccountTreeId::new(Address::repeat_byte(5)), H256::zero()); storage.set_value(erased_key, H256::repeat_byte(1)); - let mut storage = StorageWithOverrides::new(storage, &overrides); + let mut storage = apply_state_override(storage, &overrides); let balance = storage.read_value(&storage_key_for_eth_balance(&Address::repeat_byte(1))); assert_eq!(balance, H256::from_low_u64_be(1)); From 741b1ad2f765d586a2dcefad820fc27487ca9b0c Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 25 Sep 2024 16:09:23 +0300 Subject: [PATCH 03/18] Use `StorageWithOverrides` in oneshot executor --- core/lib/vm_executor/src/oneshot/mod.rs | 39 +++++++++---------- .../src/execution_sandbox/execute.rs | 18 ++++----- .../src/execution_sandbox/validate.rs | 7 ++-- core/node/consensus/src/vm.rs | 5 ++- 4 files changed, 34 insertions(+), 35 deletions(-) diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs index a5dee46df6ee..743a1bf0db1c 100644 --- a/core/lib/vm_executor/src/oneshot/mod.rs +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -17,7 +17,7 @@ use once_cell::sync::OnceCell; use zksync_multivm::{ interface::{ executor::{OneshotExecutor, TransactionValidator}, - storage::{ReadStorage, StoragePtr, StorageView, WriteStorage}, + storage::{ReadStorage, StoragePtr, StorageView, StorageWithOverrides}, tracer::{ValidationError, ValidationParams}, Call, ExecutionResult, OneshotEnv, OneshotTracingParams, OneshotTransactionExecutionResult, StoredL2BlockEnv, TxExecutionArgs, TxExecutionMode, VmExecutionMode, VmInterface, @@ -90,13 +90,13 @@ impl MainOneshotExecutor { } #[async_trait] -impl OneshotExecutor for MainOneshotExecutor +impl OneshotExecutor> for MainOneshotExecutor where S: ReadStorage + Send + 'static, { async fn inspect_transaction_with_bytecode_compression( &self, - storage: S, + storage: StorageWithOverrides, env: OneshotEnv, args: TxExecutionArgs, params: OneshotTracingParams, @@ -138,13 +138,13 @@ where } #[async_trait] -impl TransactionValidator for MainOneshotExecutor +impl TransactionValidator> for MainOneshotExecutor where S: ReadStorage + Send + 'static, { async fn validate_transaction( &self, - storage: S, + storage: StorageWithOverrides, env: OneshotEnv, tx: L2Tx, validation_params: ValidationParams, @@ -257,8 +257,8 @@ impl Vm { #[derive(Debug)] struct VmSandbox { - vm: Vm, - storage_view: StoragePtr>, + vm: Vm>, + storage_view: StoragePtr>>, transaction: Transaction, execution_latency_histogram: Option<&'static vise::Histogram>, } @@ -267,13 +267,12 @@ impl VmSandbox { /// This method is blocking. fn new( fast_vm_mode: FastVmMode, - storage: S, + mut storage: StorageWithOverrides, mut env: OneshotEnv, execution_args: TxExecutionArgs, execution_latency_histogram: Option<&'static vise::Histogram>, ) -> Self { - let mut storage_view = StorageView::new(storage); - Self::setup_storage_view(&mut storage_view, &execution_args, env.current_block); + Self::setup_storage(&mut storage, &execution_args, env.current_block); let protocol_version = env.system.version; if execution_args.adjust_pubdata_price { @@ -285,7 +284,7 @@ impl VmSandbox { ); }; - let storage_view = storage_view.to_rc_ptr(); + let storage_view = StorageView::new(storage).to_rc_ptr(); let vm = match fast_vm_mode { FastVmMode::Old => Vm::Legacy(LegacyVmInstance::new_with_specific_version( env.l1_batch, @@ -313,25 +312,25 @@ impl VmSandbox { } /// This method is blocking. - fn setup_storage_view( - storage_view: &mut StorageView, + fn setup_storage( + storage: &mut StorageWithOverrides, execution_args: &TxExecutionArgs, current_block: Option, ) { let storage_view_setup_started_at = Instant::now(); if let Some(nonce) = execution_args.enforced_nonce { let nonce_key = get_nonce_key(&execution_args.transaction.initiator_account()); - let full_nonce = storage_view.read_value(&nonce_key); + let full_nonce = storage.read_value(&nonce_key); let (_, deployment_nonce) = decompose_full_nonce(h256_to_u256(full_nonce)); let enforced_full_nonce = nonces_to_full_nonce(U256::from(nonce.0), deployment_nonce); - storage_view.set_value(nonce_key, u256_to_h256(enforced_full_nonce)); + storage.set_value(nonce_key, u256_to_h256(enforced_full_nonce)); } let payer = execution_args.transaction.payer(); let balance_key = storage_key_for_eth_balance(&payer); - let mut current_balance = h256_to_u256(storage_view.read_value(&balance_key)); + let mut current_balance = h256_to_u256(storage.read_value(&balance_key)); current_balance += execution_args.added_balance; - storage_view.set_value(balance_key, u256_to_h256(current_balance)); + storage.set_value(balance_key, u256_to_h256(current_balance)); // Reset L2 block info if necessary. if let Some(current_block) = current_block { @@ -341,13 +340,13 @@ impl VmSandbox { ); let l2_block_info = pack_block_info(current_block.number.into(), current_block.timestamp); - storage_view.set_value(l2_block_info_key, u256_to_h256(l2_block_info)); + storage.set_value(l2_block_info_key, u256_to_h256(l2_block_info)); let l2_block_txs_rolling_hash_key = StorageKey::new( AccountTreeId::new(SYSTEM_CONTEXT_ADDRESS), SYSTEM_CONTEXT_CURRENT_TX_ROLLING_HASH_POSITION, ); - storage_view.set_value( + storage.set_value( l2_block_txs_rolling_hash_key, current_block.txs_rolling_hash, ); @@ -362,7 +361,7 @@ impl VmSandbox { pub(super) fn apply(mut self, apply_fn: F) -> T where - F: FnOnce(&mut Vm, Transaction) -> T, + F: FnOnce(&mut Vm>, Transaction) -> T, { let tx_id = format!( "{:?}-{}", diff --git a/core/node/api_server/src/execution_sandbox/execute.rs b/core/node/api_server/src/execution_sandbox/execute.rs index d974f2e9aa1d..b6cfd84675a9 100644 --- a/core/node/api_server/src/execution_sandbox/execute.rs +++ b/core/node/api_server/src/execution_sandbox/execute.rs @@ -8,23 +8,23 @@ use tokio::runtime::Handle; use zksync_dal::{Connection, Core}; use zksync_multivm::interface::{ executor::{OneshotExecutor, TransactionValidator}, - storage::ReadStorage, + storage::{ReadStorage, StorageWithOverrides}, tracer::{ValidationError, ValidationParams}, Call, OneshotEnv, OneshotTracingParams, OneshotTransactionExecutionResult, TransactionExecutionMetrics, TxExecutionArgs, VmExecutionResultAndLogs, }; use zksync_state::{PostgresStorage, PostgresStorageCaches}; use zksync_types::{ - api::state_override::StateOverride, fee_model::BatchFeeInput, l2::L2Tx, Transaction, + api::state_override::StateOverride, fee_model::BatchFeeInput, l2::L2Tx, vm::FastVmMode, + Transaction, }; use zksync_vm_executor::oneshot::{MainOneshotExecutor, MockOneshotExecutor}; use super::{ - storage::StorageWithOverrides, vm_metrics::{self, SandboxStage}, BlockArgs, VmPermit, SANDBOX_METRICS, }; -use crate::tx_sender::SandboxExecutorOptions; +use crate::{execution_sandbox::storage::apply_state_override, tx_sender::SandboxExecutorOptions}; /// Action that can be executed by [`SandboxExecutor`]. #[derive(Debug)] @@ -144,7 +144,7 @@ impl SandboxExecutor { .await?; let state_override = state_override.unwrap_or_default(); - let storage = StorageWithOverrides::new(storage, &state_override); + let storage = apply_state_override(storage, &state_override); let (execution_args, tracing_params) = action.into_parts(); let result = self .inspect_transaction_with_bytecode_compression( @@ -239,13 +239,13 @@ impl SandboxExecutor { } #[async_trait] -impl OneshotExecutor for SandboxExecutor +impl OneshotExecutor> for SandboxExecutor where S: ReadStorage + Send + 'static, { async fn inspect_transaction_with_bytecode_compression( &self, - storage: S, + storage: StorageWithOverrides, env: OneshotEnv, args: TxExecutionArgs, tracing_params: OneshotTracingParams, @@ -276,13 +276,13 @@ where } #[async_trait] -impl TransactionValidator for SandboxExecutor +impl TransactionValidator> for SandboxExecutor where S: ReadStorage + Send + 'static, { async fn validate_transaction( &self, - storage: S, + storage: StorageWithOverrides, env: OneshotEnv, tx: L2Tx, validation_params: ValidationParams, diff --git a/core/node/api_server/src/execution_sandbox/validate.rs b/core/node/api_server/src/execution_sandbox/validate.rs index 9a3c88f8bf0c..758547abbd6e 100644 --- a/core/node/api_server/src/execution_sandbox/validate.rs +++ b/core/node/api_server/src/execution_sandbox/validate.rs @@ -5,16 +5,15 @@ use tracing::Instrument; use zksync_dal::{Connection, Core, CoreDal}; use zksync_multivm::interface::{ executor::TransactionValidator, + storage::StorageWithOverrides, tracer::{ValidationError as RawValidationError, ValidationParams}, }; use zksync_types::{ - api::state_override::StateOverride, fee_model::BatchFeeInput, l2::L2Tx, Address, - TRUSTED_ADDRESS_SLOTS, TRUSTED_TOKEN_SLOTS, + fee_model::BatchFeeInput, l2::L2Tx, Address, TRUSTED_ADDRESS_SLOTS, TRUSTED_TOKEN_SLOTS, }; use super::{ execute::{SandboxAction, SandboxExecutor}, - storage::StorageWithOverrides, vm_metrics::{SandboxStage, EXECUTION_METRICS, SANDBOX_METRICS}, BlockArgs, VmPermit, }; @@ -57,7 +56,7 @@ impl SandboxExecutor { let SandboxAction::Execution { tx, .. } = action else { unreachable!(); // by construction }; - let storage = StorageWithOverrides::new(storage, &StateOverride::default()); + let storage = StorageWithOverrides::new(storage); let stage_latency = SANDBOX_METRICS.sandbox[&SandboxStage::Validation].start(); let validation_result = self diff --git a/core/node/consensus/src/vm.rs b/core/node/consensus/src/vm.rs index 149e6b3ccb03..b0711f4cd010 100644 --- a/core/node/consensus/src/vm.rs +++ b/core/node/consensus/src/vm.rs @@ -7,7 +7,8 @@ use zksync_system_constants::DEFAULT_L2_TX_GAS_PER_PUBDATA_BYTE; use zksync_types::{ethabi, fee::Fee, l2::L2Tx, AccountTreeId, L2ChainId, Nonce, U256}; use zksync_vm_executor::oneshot::{CallOrExecute, MainOneshotExecutor, OneshotEnvParameters}; use zksync_vm_interface::{ - executor::OneshotExecutor, ExecutionResult, OneshotTracingParams, TxExecutionArgs, + executor::OneshotExecutor, storage::StorageWithOverrides, ExecutionResult, + OneshotTracingParams, TxExecutionArgs, }; use crate::{abi, storage::ConnectionPool}; @@ -84,7 +85,7 @@ impl VM { let output = ctx .wait(self.executor.inspect_transaction_with_bytecode_compression( - storage, + StorageWithOverrides::new(storage), env, TxExecutionArgs::for_eth_call(tx), OneshotTracingParams::default(), From 50738dfa82df1c75fa47b1ff45f327078b925201 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 2 Oct 2024 11:32:36 +0300 Subject: [PATCH 04/18] Split fast VM modes for different ops --- core/lib/vm_executor/src/oneshot/mod.rs | 56 ++++++++++++++++--- .../src/execution_sandbox/execute.rs | 4 +- core/node/api_server/src/tx_sender/mod.rs | 12 +++- .../api_server/src/tx_sender/tests/mod.rs | 3 +- 4 files changed, 62 insertions(+), 13 deletions(-) diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs index 743a1bf0db1c..6b7e19478915 100644 --- a/core/lib/vm_executor/src/oneshot/mod.rs +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -51,10 +51,48 @@ mod env; mod metrics; mod mock; +/// Fast VM modes utilized for different kinds of operations. +#[derive(Debug, Clone, Copy, Default)] +pub struct OneshotExecutorVmModes { + /// Mode used for gas estimation. + pub gas_estimation: FastVmMode, + /// Mode used for `eth_call`s. + pub call: FastVmMode, + /// Mode used for transaction execution (doesn't include the validation step). + pub tx_execution: FastVmMode, +} + +/// Sets all modes to the specified value. +impl From for OneshotExecutorVmModes { + fn from(mode: FastVmMode) -> Self { + Self { + gas_estimation: mode, + call: mode, + tx_execution: mode, + } + } +} + +impl OneshotExecutorVmModes { + fn uses_fast_vm(&self) -> bool { + !matches!(self.gas_estimation, FastVmMode::Old) + || !matches!(self.call, FastVmMode::Old) + || !matches!(self.tx_execution, FastVmMode::Old) + } + + fn get(&self, mode: TxExecutionMode) -> FastVmMode { + match mode { + TxExecutionMode::VerifyExecute => self.tx_execution, + TxExecutionMode::EthCall => self.call, + TxExecutionMode::EstimateFee => self.gas_estimation, + } + } +} + /// Main [`OneshotExecutor`] implementation used by the API server. -#[derive(Debug, Default)] +#[derive(Debug)] pub struct MainOneshotExecutor { - fast_vm_mode: FastVmMode, + fast_vm_modes: OneshotExecutorVmModes, missed_storage_invocation_limit: usize, execution_latency_histogram: Option<&'static vise::Histogram>, } @@ -64,20 +102,20 @@ impl MainOneshotExecutor { /// The limit is applied for calls and gas estimations, but not during transaction validation. pub fn new(missed_storage_invocation_limit: usize) -> Self { Self { - fast_vm_mode: FastVmMode::Old, + fast_vm_modes: OneshotExecutorVmModes::default(), missed_storage_invocation_limit, execution_latency_histogram: None, } } - /// Sets the fast VM mode used by this executor. - pub fn set_fast_vm_mode(&mut self, fast_vm_mode: FastVmMode) { - if !matches!(fast_vm_mode, FastVmMode::Old) { + /// Sets the fast VM modes used by this executor. + pub fn set_fast_vm_modes(&mut self, fast_vm_modes: OneshotExecutorVmModes) { + if fast_vm_modes.uses_fast_vm() { tracing::warn!( - "Running new VM with mode {fast_vm_mode:?}; this can lead to incorrect node behavior" + "Running new VM with modes {fast_vm_modes:?}; this can lead to incorrect node behavior" ); } - self.fast_vm_mode = fast_vm_mode; + self.fast_vm_modes = fast_vm_modes; } /// Sets a histogram for measuring VM execution latency. @@ -112,7 +150,7 @@ where let fast_vm_mode = if params.trace_calls { FastVmMode::Old // the fast VM doesn't support call tracing } else { - self.fast_vm_mode + self.fast_vm_modes.get(env.system.execution_mode) }; tokio::task::spawn_blocking(move || { diff --git a/core/node/api_server/src/execution_sandbox/execute.rs b/core/node/api_server/src/execution_sandbox/execute.rs index b6cfd84675a9..be529af7b072 100644 --- a/core/node/api_server/src/execution_sandbox/execute.rs +++ b/core/node/api_server/src/execution_sandbox/execute.rs @@ -15,8 +15,7 @@ use zksync_multivm::interface::{ }; use zksync_state::{PostgresStorage, PostgresStorageCaches}; use zksync_types::{ - api::state_override::StateOverride, fee_model::BatchFeeInput, l2::L2Tx, vm::FastVmMode, - Transaction, + api::state_override::StateOverride, fee_model::BatchFeeInput, l2::L2Tx, Transaction, }; use zksync_vm_executor::oneshot::{MainOneshotExecutor, MockOneshotExecutor}; @@ -109,6 +108,7 @@ impl SandboxExecutor { missed_storage_invocation_limit: usize, ) -> Self { let mut executor = MainOneshotExecutor::new(missed_storage_invocation_limit); + executor.set_fast_vm_modes(options.fast_vm_modes); executor .set_execution_latency_histogram(&SANDBOX_METRICS.sandbox[&SandboxStage::Execution]); Self { diff --git a/core/node/api_server/src/tx_sender/mod.rs b/core/node/api_server/src/tx_sender/mod.rs index ad8e38ef3cc2..83ec880f0f6d 100644 --- a/core/node/api_server/src/tx_sender/mod.rs +++ b/core/node/api_server/src/tx_sender/mod.rs @@ -25,11 +25,14 @@ use zksync_types::{ l2::{error::TxCheckError::TxDuplication, L2Tx}, transaction_request::CallOverrides, utils::storage_key_for_eth_balance, + vm::FastVmMode, AccountTreeId, Address, L2ChainId, Nonce, ProtocolVersionId, Transaction, H160, H256, MAX_NEW_FACTORY_DEPS, U256, }; use zksync_utils::h256_to_u256; -use zksync_vm_executor::oneshot::{CallOrExecute, EstimateGas, OneshotEnvParameters}; +use zksync_vm_executor::oneshot::{ + CallOrExecute, EstimateGas, OneshotEnvParameters, OneshotExecutorVmModes, +}; pub(super) use self::{gas_estimation::BinarySearchKind, result::SubmitTxError}; use self::{master_pool_sink::MasterPoolSink, result::ApiCallResult, tx_sink::TxSink}; @@ -87,6 +90,7 @@ pub async fn build_tx_sender( /// Oneshot executor options used by the API server sandbox. #[derive(Debug)] pub struct SandboxExecutorOptions { + pub(crate) fast_vm_modes: OneshotExecutorVmModes, /// Env parameters to be used when estimating gas. pub(crate) estimate_gas: OneshotEnvParameters, /// Env parameters to be used when performing `eth_call` requests. @@ -103,6 +107,7 @@ impl SandboxExecutorOptions { validation_computational_gas_limit: u32, ) -> anyhow::Result { Ok(Self { + fast_vm_modes: FastVmMode::Old.into(), estimate_gas: OneshotEnvParameters::for_gas_estimation(chain_id, operator_account) .await?, eth_call: OneshotEnvParameters::for_execution( @@ -114,6 +119,11 @@ impl SandboxExecutorOptions { }) } + /// Sets the fast VM modes used by this executor. + pub fn set_fast_vm_modes(&mut self, fast_vm_modes: OneshotExecutorVmModes) { + self.fast_vm_modes = fast_vm_modes; + } + pub(crate) async fn mock() -> Self { Self::new(L2ChainId::default(), AccountTreeId::default(), u32::MAX) .await diff --git a/core/node/api_server/src/tx_sender/tests/mod.rs b/core/node/api_server/src/tx_sender/tests/mod.rs index 3d48e320abcb..a55c7843e24a 100644 --- a/core/node/api_server/src/tx_sender/tests/mod.rs +++ b/core/node/api_server/src/tx_sender/tests/mod.rs @@ -145,13 +145,14 @@ async fn create_real_tx_sender(pool: ConnectionPool) -> TxSender { drop(storage); let genesis_config = genesis_params.config(); - let executor_options = SandboxExecutorOptions::new( + let mut executor_options = SandboxExecutorOptions::new( genesis_config.l2_chain_id, AccountTreeId::new(genesis_config.fee_account), u32::MAX, ) .await .unwrap(); + executor_options.set_fast_vm_modes(FastVmMode::Shadow.into()); let pg_caches = PostgresStorageCaches::new(1, 1); let tx_executor = SandboxExecutor::real(executor_options, pg_caches, usize::MAX); From 053d51086213f5a33f5ba99c2b0bae043e394957 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 8 Oct 2024 11:01:14 +0300 Subject: [PATCH 05/18] Do not panic on divergence in oneshot executor --- core/lib/multivm/src/lib.rs | 2 +- core/lib/multivm/src/vm_instance.rs | 12 +++++- core/lib/vm_executor/src/oneshot/mod.rs | 56 +++++++++++++++++++------ 3 files changed, 54 insertions(+), 16 deletions(-) diff --git a/core/lib/multivm/src/lib.rs b/core/lib/multivm/src/lib.rs index e171a78e1794..520274c14ae0 100644 --- a/core/lib/multivm/src/lib.rs +++ b/core/lib/multivm/src/lib.rs @@ -16,7 +16,7 @@ pub use crate::{ 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::{FastVmInstance, LegacyVmInstance}, + vm_instance::{is_supported_by_fast_vm, FastVmInstance, LegacyVmInstance}, }; mod glue; diff --git a/core/lib/multivm/src/vm_instance.rs b/core/lib/multivm/src/vm_instance.rs index ac5693b61619..7e17e0db1d76 100644 --- a/core/lib/multivm/src/vm_instance.rs +++ b/core/lib/multivm/src/vm_instance.rs @@ -1,6 +1,6 @@ use std::mem; -use zksync_types::{vm::VmVersion, Transaction}; +use zksync_types::{vm::VmVersion, ProtocolVersionId, Transaction}; use zksync_vm2::interface::Tracer; use crate::{ @@ -225,7 +225,7 @@ pub type ShadowedFastVm = ShadowVm< /// Fast VM variants. #[derive(Debug)] -pub enum FastVmInstance { +pub enum FastVmInstance { /// Fast VM running in isolation. Fast(crate::vm_fast::Vm, Tr>), /// Fast VM shadowed by the latest legacy VM. @@ -328,3 +328,11 @@ impl FastVmInstance { Self::Shadowed(ShadowedFastVm::new(l1_batch_env, system_env, storage_view)) } } + +/// Checks whether the protocol version is supported by the fast VM. +pub fn is_supported_by_fast_vm(protocol_version: ProtocolVersionId) -> bool { + matches!( + protocol_version.into(), + VmVersion::Vm1_5_0IncreasedBootloaderMemory + ) +} diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs index 6b7e19478915..c7f89466fb19 100644 --- a/core/lib/vm_executor/src/oneshot/mod.rs +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -19,9 +19,12 @@ use zksync_multivm::{ executor::{OneshotExecutor, TransactionValidator}, storage::{ReadStorage, StoragePtr, StorageView, StorageWithOverrides}, tracer::{ValidationError, ValidationParams}, + utils::{DivergenceHandler, ShadowVm}, Call, ExecutionResult, OneshotEnv, OneshotTracingParams, OneshotTransactionExecutionResult, - StoredL2BlockEnv, TxExecutionArgs, TxExecutionMode, VmExecutionMode, VmInterface, + StoredL2BlockEnv, TxExecutionArgs, TxExecutionMode, VmExecutionMode, VmFactory, + VmInterface, }, + is_supported_by_fast_vm, tracers::{CallTracer, StorageInvocations, TracerDispatcher, ValidationTracer}, utils::adjust_pubdata_price_for_tx, vm_latest::{HistoryDisabled, HistoryEnabled}, @@ -93,6 +96,7 @@ impl OneshotExecutorVmModes { #[derive(Debug)] pub struct MainOneshotExecutor { fast_vm_modes: OneshotExecutorVmModes, + panic_on_divergence: bool, missed_storage_invocation_limit: usize, execution_latency_histogram: Option<&'static vise::Histogram>, } @@ -103,6 +107,7 @@ impl MainOneshotExecutor { pub fn new(missed_storage_invocation_limit: usize) -> Self { Self { fast_vm_modes: OneshotExecutorVmModes::default(), + panic_on_divergence: false, missed_storage_invocation_limit, execution_latency_histogram: None, } @@ -118,6 +123,11 @@ impl MainOneshotExecutor { self.fast_vm_modes = fast_vm_modes; } + /// Causes the VM to panic on divergence whenever it executes in the shadow mode. By default, a divergence is logged on `ERROR` level. + pub fn panic_on_divergence(&mut self) { + self.panic_on_divergence = true; + } + /// Sets a histogram for measuring VM execution latency. pub fn set_execution_latency_histogram( &mut self, @@ -125,6 +135,18 @@ impl MainOneshotExecutor { ) { self.execution_latency_histogram = Some(histogram); } + + fn select_fast_vm_mode( + &self, + env: &OneshotEnv, + tracing_params: &OneshotTracingParams, + ) -> FastVmMode { + if tracing_params.trace_calls || !is_supported_by_fast_vm(env.system.version) { + FastVmMode::Old // the fast VM doesn't support call tracing or old protocol versions + } else { + self.fast_vm_modes.get(env.system.execution_mode) + } + } } #[async_trait] @@ -137,7 +159,7 @@ where storage: StorageWithOverrides, env: OneshotEnv, args: TxExecutionArgs, - params: OneshotTracingParams, + tracing_params: OneshotTracingParams, ) -> anyhow::Result { let missed_storage_invocation_limit = match env.system.execution_mode { // storage accesses are not limited for tx validation @@ -147,15 +169,13 @@ where } }; let execution_latency_histogram = self.execution_latency_histogram; - let fast_vm_mode = if params.trace_calls { - FastVmMode::Old // the fast VM doesn't support call tracing - } else { - self.fast_vm_modes.get(env.system.execution_mode) - }; + let fast_vm_mode = self.select_fast_vm_mode(&env, &tracing_params); + let panic_on_divergence = self.panic_on_divergence; tokio::task::spawn_blocking(move || { let executor = VmSandbox::new( fast_vm_mode, + panic_on_divergence, storage, env, args, @@ -164,7 +184,7 @@ where executor.apply(|vm, transaction| { vm.inspect_transaction_with_bytecode_compression( missed_storage_invocation_limit, - params, + tracing_params, transaction, true, ) @@ -204,6 +224,7 @@ where let executor = VmSandbox::new( FastVmMode::Old, + false, storage, env, TxExecutionArgs::for_validation(tx), @@ -305,6 +326,7 @@ impl VmSandbox { /// This method is blocking. fn new( fast_vm_mode: FastVmMode, + panic_on_divergence: bool, mut storage: StorageWithOverrides, mut env: OneshotEnv, execution_args: TxExecutionArgs, @@ -312,6 +334,7 @@ impl VmSandbox { ) -> Self { Self::setup_storage(&mut storage, &execution_args, env.current_block); + let mode = env.system.execution_mode; let protocol_version = env.system.version; if execution_args.adjust_pubdata_price { env.l1_batch.fee_input = adjust_pubdata_price_for_tx( @@ -335,12 +358,19 @@ impl VmSandbox { env.system, storage_view.clone(), )), - FastVmMode::Shadow => Vm::Fast(FastVmInstance::shadowed( - env.l1_batch, - env.system, - storage_view.clone(), - )), + FastVmMode::Shadow => { + let mut vm = ShadowVm::new(env.l1_batch, env.system, storage_view.clone()); + if !panic_on_divergence { + let transaction = format!("{:?}", execution_args.transaction); + let handler = DivergenceHandler::new(move |errors, _| { + tracing::error!(transaction, ?mode, "{errors}"); + }); + vm.set_divergence_handler(handler); + } + Vm::Fast(FastVmInstance::Shadowed(vm)) + } }; + Self { vm, storage_view, From d9e175cefdb2e02aa8ecaff095af3d8e76b710df Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 8 Oct 2024 11:01:37 +0300 Subject: [PATCH 06/18] Add basic tests for oneshot executor --- Cargo.lock | 2 + core/lib/vm_executor/Cargo.toml | 4 + core/lib/vm_executor/src/oneshot/mod.rs | 2 + core/lib/vm_executor/src/oneshot/tests.rs | 192 ++++++++++++++++++ core/lib/vm_interface/src/types/inputs/mod.rs | 2 +- 5 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 core/lib/vm_executor/src/oneshot/tests.rs diff --git a/Cargo.lock b/Cargo.lock index 5073188d6321..15b895da5943 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11313,8 +11313,10 @@ name = "zksync_vm_executor" version = "0.1.0" dependencies = [ "anyhow", + "assert_matches", "async-trait", "once_cell", + "test-casing", "tokio", "tracing", "vise", diff --git a/core/lib/vm_executor/Cargo.toml b/core/lib/vm_executor/Cargo.toml index 089c2a9bcca7..06a531252c54 100644 --- a/core/lib/vm_executor/Cargo.toml +++ b/core/lib/vm_executor/Cargo.toml @@ -23,3 +23,7 @@ tokio.workspace = true anyhow.workspace = true tracing.workspace = true vise.workspace = true + +[dev-dependencies] +assert_matches.workspace = true +test-casing.workspace = true diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs index c7f89466fb19..f96cabb3f4a5 100644 --- a/core/lib/vm_executor/src/oneshot/mod.rs +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -53,6 +53,8 @@ mod contracts; mod env; mod metrics; mod mock; +#[cfg(test)] +mod tests; /// Fast VM modes utilized for different kinds of operations. #[derive(Debug, Clone, Copy, Default)] diff --git a/core/lib/vm_executor/src/oneshot/tests.rs b/core/lib/vm_executor/src/oneshot/tests.rs new file mode 100644 index 000000000000..7c72638f0b62 --- /dev/null +++ b/core/lib/vm_executor/src/oneshot/tests.rs @@ -0,0 +1,192 @@ +//! Oneshot executor tests. + +use assert_matches::assert_matches; +use once_cell::sync::Lazy; +use test_casing::{test_casing, Product}; +use zksync_contracts::BaseSystemContracts; +use zksync_multivm::{ + interface::{storage::InMemoryStorage, L1BatchEnv, L2BlockEnv, SystemEnv}, + utils::derive_base_fee_and_gas_per_pubdata, + vm_latest::constants::BATCH_COMPUTATIONAL_GAS_LIMIT, + zk_evm_latest::ethereum_types::Address, +}; +use zksync_types::{ + block::L2BlockHasher, fee::Fee, fee_model::BatchFeeInput, transaction_request::PaymasterParams, + K256PrivateKey, L1BatchNumber, L2BlockNumber, L2ChainId, ProtocolVersionId, H256, + ZKPORTER_IS_AVAILABLE, +}; +use zksync_utils::bytecode::hash_bytecode; + +use super::*; + +static BASE_SYSTEM_CONTRACTS: Lazy = + Lazy::new(BaseSystemContracts::load_from_disk); + +const EXEC_MODES: [TxExecutionMode; 3] = [ + TxExecutionMode::EstimateFee, + TxExecutionMode::EthCall, + TxExecutionMode::VerifyExecute, +]; +const FAST_VM_MODES: [FastVmMode; 3] = [FastVmMode::Old, FastVmMode::New, FastVmMode::Shadow]; + +fn default_system_env(execution_mode: TxExecutionMode) -> SystemEnv { + SystemEnv { + zk_porter_available: ZKPORTER_IS_AVAILABLE, + version: ProtocolVersionId::latest(), + base_system_smart_contracts: BASE_SYSTEM_CONTRACTS.clone(), + bootloader_gas_limit: BATCH_COMPUTATIONAL_GAS_LIMIT, + execution_mode, + default_validation_computational_gas_limit: BATCH_COMPUTATIONAL_GAS_LIMIT, + chain_id: L2ChainId::default(), + } +} + +fn default_l1_batch_env(number: u32) -> L1BatchEnv { + L1BatchEnv { + previous_batch_hash: Some(H256::zero()), + number: L1BatchNumber(number), + timestamp: number.into(), + fee_account: Address::repeat_byte(0x22), + enforced_base_fee: None, + first_l2_block: L2BlockEnv { + number, + timestamp: number.into(), + prev_block_hash: L2BlockHasher::legacy_hash(L2BlockNumber(number - 1)), + max_virtual_blocks_to_create: 1, + }, + fee_input: BatchFeeInput::sensible_l1_pegged_default(), + } +} + +fn create_l2_transaction(value: U256, nonce: Nonce) -> L2Tx { + let (max_fee_per_gas, gas_per_pubdata_limit) = derive_base_fee_and_gas_per_pubdata( + BatchFeeInput::sensible_l1_pegged_default(), + ProtocolVersionId::latest().into(), + ); + let fee = Fee { + gas_limit: 10_000_000.into(), + max_fee_per_gas: max_fee_per_gas.into(), + max_priority_fee_per_gas: 0_u64.into(), + gas_per_pubdata_limit: gas_per_pubdata_limit.into(), + }; + L2Tx::new_signed( + Some(Address::random()), + vec![], + nonce, + fee, + value, + L2ChainId::default(), + &K256PrivateKey::random(), + vec![], + PaymasterParams::default(), + ) + .unwrap() +} + +#[test] +fn selecting_vm_for_execution() { + let mut executor = MainOneshotExecutor::new(usize::MAX); + executor.set_fast_vm_modes(FastVmMode::New.into()); + + for exec_mode in EXEC_MODES { + let env = OneshotEnv { + system: default_system_env(exec_mode), + l1_batch: default_l1_batch_env(1), + current_block: None, + }; + let mode = executor.select_fast_vm_mode(&env, &OneshotTracingParams::default()); + assert_matches!(mode, FastVmMode::New); + + // Tracing calls is not supported by the new VM. + let mode = executor.select_fast_vm_mode(&env, &OneshotTracingParams { trace_calls: true }); + assert_matches!(mode, FastVmMode::Old); + + // Old protocol versions are not supported either. + let mut old_env = env.clone(); + old_env.system.version = ProtocolVersionId::Version22; + let mode = executor.select_fast_vm_mode(&old_env, &OneshotTracingParams::default()); + assert_matches!(mode, FastVmMode::Old); + } + + executor.set_fast_vm_modes(OneshotExecutorVmModes { + gas_estimation: FastVmMode::New, + ..OneshotExecutorVmModes::default() + }); + + let mut env = OneshotEnv { + system: default_system_env(TxExecutionMode::EstimateFee), + l1_batch: default_l1_batch_env(1), + current_block: None, + }; + let mode = executor.select_fast_vm_mode(&env, &OneshotTracingParams::default()); + assert_matches!(mode, FastVmMode::New); + + for exec_mode in [TxExecutionMode::VerifyExecute, TxExecutionMode::EthCall] { + env.system.execution_mode = exec_mode; + let mode = executor.select_fast_vm_mode(&env, &OneshotTracingParams::default()); + assert_matches!(mode, FastVmMode::Old); + } +} + +#[test] +fn setting_up_nonce_and_balance_in_storage() { + let mut storage = StorageWithOverrides::new(InMemoryStorage::default()); + let tx = create_l2_transaction(1_000_000_000.into(), Nonce(1)); + let execution_args = TxExecutionArgs::for_gas_estimate(tx.clone().into()); + VmSandbox::setup_storage(&mut storage, &execution_args, None); + + // Check the overridden nonce and balance. + let nonce_key = get_nonce_key(&tx.initiator_account()); + assert_eq!(storage.read_value(&nonce_key), H256::from_low_u64_be(1)); + let balance_key = storage_key_for_eth_balance(&tx.initiator_account()); + let expected_added_balance = tx.common_data.fee.gas_limit * tx.common_data.fee.max_fee_per_gas; + assert_eq!( + storage.read_value(&balance_key), + u256_to_h256(expected_added_balance) + ); + + let mut storage = InMemoryStorage::default(); + storage.set_value(balance_key, H256::from_low_u64_be(2_000_000_000)); + let mut storage = StorageWithOverrides::new(storage); + VmSandbox::setup_storage(&mut storage, &execution_args, None); + + assert_eq!( + storage.read_value(&balance_key), + u256_to_h256(expected_added_balance + U256::from(2_000_000_000)) + ); +} + +#[test_casing(9, Product((EXEC_MODES, FAST_VM_MODES)))] +#[tokio::test] +async fn inspecting_transfer(exec_mode: TxExecutionMode, fast_vm_mode: FastVmMode) { + let tx = create_l2_transaction(1_000_000_000.into(), Nonce(0)); + let mut storage = InMemoryStorage::with_system_contracts(hash_bytecode); + storage.set_value( + storage_key_for_eth_balance(&tx.initiator_account()), + u256_to_h256(u64::MAX.into()), + ); + let storage = StorageWithOverrides::new(storage); + + let l1_batch = default_l1_batch_env(1); + let env = OneshotEnv { + system: default_system_env(exec_mode), + current_block: Some(StoredL2BlockEnv { + number: l1_batch.first_l2_block.number - 1, + timestamp: l1_batch.first_l2_block.timestamp - 1, + txs_rolling_hash: H256::zero(), + }), + l1_batch, + }; + let args = TxExecutionArgs::for_gas_estimate(tx.into()); + let tracing = OneshotTracingParams::default(); + + let mut executor = MainOneshotExecutor::new(usize::MAX); + executor.set_fast_vm_modes(fast_vm_mode.into()); + let result = executor + .inspect_transaction_with_bytecode_compression(storage, env, args, tracing) + .await + .unwrap(); + result.compression_result.unwrap(); + let exec_result = result.tx_result.result; + assert!(!exec_result.is_failed(), "{exec_result:?}"); +} diff --git a/core/lib/vm_interface/src/types/inputs/mod.rs b/core/lib/vm_interface/src/types/inputs/mod.rs index 24f58ae72f16..ff395dd9d16a 100644 --- a/core/lib/vm_interface/src/types/inputs/mod.rs +++ b/core/lib/vm_interface/src/types/inputs/mod.rs @@ -15,7 +15,7 @@ mod l2_block; mod system_env; /// Full environment for oneshot transaction / call execution. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct OneshotEnv { /// System environment. pub system: SystemEnv, From 86fe4a807bedbd92e0460c9f936e1047cbc47644 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 8 Oct 2024 11:08:01 +0300 Subject: [PATCH 07/18] Fix metrics reporting for new VM --- core/lib/vm_executor/src/oneshot/metrics.rs | 16 +++++++++--- core/lib/vm_executor/src/oneshot/mod.rs | 29 ++++++++++++++------- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/core/lib/vm_executor/src/oneshot/metrics.rs b/core/lib/vm_executor/src/oneshot/metrics.rs index 475463300f16..13a832ee3c89 100644 --- a/core/lib/vm_executor/src/oneshot/metrics.rs +++ b/core/lib/vm_executor/src/oneshot/metrics.rs @@ -50,7 +50,7 @@ pub(super) fn report_vm_memory_metrics( tx_id: &str, memory_metrics: &VmMemoryMetrics, vm_execution_took: Duration, - storage_metrics: &StorageViewStats, + storage_stats: &StorageViewStats, ) { MEMORY_METRICS.event_sink_size[&SizeType::Inner].observe(memory_metrics.event_sink_inner); MEMORY_METRICS.event_sink_size[&SizeType::History].observe(memory_metrics.event_sink_history); @@ -65,10 +65,18 @@ pub(super) fn report_vm_memory_metrics( MEMORY_METRICS .storage_view_cache_size - .observe(storage_metrics.cache_size); + .observe(storage_stats.cache_size); MEMORY_METRICS .full - .observe(memory_metrics.full_size() + storage_metrics.cache_size); + .observe(memory_metrics.full_size() + storage_stats.cache_size); - STORAGE_METRICS.observe(&format!("Tx {tx_id}"), vm_execution_took, storage_metrics); + report_vm_storage_metrics(tx_id, vm_execution_took, storage_stats); +} + +pub(super) fn report_vm_storage_metrics( + tx_id: &str, + vm_execution_took: Duration, + storage_stats: &StorageViewStats, +) { + STORAGE_METRICS.observe(&format!("Tx {tx_id}"), vm_execution_took, storage_stats); } diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs index f96cabb3f4a5..a48f2dfcd610 100644 --- a/core/lib/vm_executor/src/oneshot/mod.rs +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -446,15 +446,26 @@ impl VmSandbox { if let Some(histogram) = self.execution_latency_histogram { histogram.observe(vm_execution_took); } - if let Vm::Legacy(vm) = &self.vm { - let memory_metrics = vm.record_vm_memory_metrics(); - metrics::report_vm_memory_metrics( - &tx_id, - &memory_metrics, - vm_execution_took, - &self.storage_view.borrow().stats(), - ); - // FIXME: Memory metrics don't work for the fast VM yet + + match &self.vm { + Vm::Legacy(vm) => { + let memory_metrics = vm.record_vm_memory_metrics(); + metrics::report_vm_memory_metrics( + &tx_id, + &memory_metrics, + vm_execution_took, + &self.storage_view.borrow().stats(), + ); + } + Vm::Fast(_) => { + // The new VM implementation doesn't have the same memory model as old ones, so it doesn't report memory metrics, + // only storage-related ones. + metrics::report_vm_storage_metrics( + &format!("Tx {tx_id}"), + vm_execution_took, + &self.storage_view.borrow().stats(), + ); + } } result } From b14f307f2e4a75d40e549b28aeb4af51e935810e Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 8 Oct 2024 11:31:37 +0300 Subject: [PATCH 08/18] Add config option for gas estimation VM mode --- core/bin/zksync_server/src/node_builder.rs | 9 ++++++-- core/lib/config/src/configs/experimental.rs | 4 ++++ core/lib/config/src/testonly.rs | 1 + core/lib/env_config/src/vm_runner.rs | 5 +++++ core/lib/protobuf_config/src/experimental.rs | 21 +++++++++++++------ .../src/proto/config/experimental.proto | 1 + .../src/execution_sandbox/execute.rs | 2 ++ .../layers/web3_api/tx_sender.rs | 14 +++++++++++-- 8 files changed, 47 insertions(+), 10 deletions(-) diff --git a/core/bin/zksync_server/src/node_builder.rs b/core/bin/zksync_server/src/node_builder.rs index 4600b0f9e543..fd9236e5613f 100644 --- a/core/bin/zksync_server/src/node_builder.rs +++ b/core/bin/zksync_server/src/node_builder.rs @@ -308,10 +308,12 @@ impl MainNodeBuilder { initial_writes_cache_size: rpc_config.initial_writes_cache_size() as u64, latest_values_cache_size: rpc_config.latest_values_cache_size() as u64, }; + let vm_config = try_load_config!(self.configs.experimental_vm_config); // On main node we always use master pool sink. self.node.add_layer(MasterPoolSinkLayer); - self.node.add_layer(TxSenderLayer::new( + + let layer = TxSenderLayer::new( TxSenderConfig::new( &sk_config, &rpc_config, @@ -322,7 +324,10 @@ impl MainNodeBuilder { ), postgres_storage_caches_config, rpc_config.vm_concurrency_limit(), - )); + ); + let layer = + layer.with_gas_estimation_vm_mode(vm_config.api_fast_vm_mode_for_gas_estimation); + self.node.add_layer(layer); Ok(self) } diff --git a/core/lib/config/src/configs/experimental.rs b/core/lib/config/src/configs/experimental.rs index 618cfd3d388c..030b278581e6 100644 --- a/core/lib/config/src/configs/experimental.rs +++ b/core/lib/config/src/configs/experimental.rs @@ -106,4 +106,8 @@ pub struct ExperimentalVmConfig { /// the new VM doesn't produce call traces and can diverge from the old VM! #[serde(default)] pub state_keeper_fast_vm_mode: FastVmMode, + + /// Fast VM mode to use for gas estimation in the API server. + #[serde(default)] + pub api_fast_vm_mode_for_gas_estimation: FastVmMode, } diff --git a/core/lib/config/src/testonly.rs b/core/lib/config/src/testonly.rs index 1d90034410b6..bf155b53f195 100644 --- a/core/lib/config/src/testonly.rs +++ b/core/lib/config/src/testonly.rs @@ -326,6 +326,7 @@ impl Distribution for EncodeDist { configs::ExperimentalVmConfig { playground: self.sample(rng), state_keeper_fast_vm_mode: gen_fast_vm_mode(rng), + api_fast_vm_mode_for_gas_estimation: gen_fast_vm_mode(rng), } } } diff --git a/core/lib/env_config/src/vm_runner.rs b/core/lib/env_config/src/vm_runner.rs index 730a79dd340a..60fafcc3d3a6 100644 --- a/core/lib/env_config/src/vm_runner.rs +++ b/core/lib/env_config/src/vm_runner.rs @@ -55,6 +55,7 @@ mod tests { let mut lock = MUTEX.lock(); let config = r#" EXPERIMENTAL_VM_STATE_KEEPER_FAST_VM_MODE=new + EXPERIMENTAL_VM_API_FAST_VM_MODE_FOR_GAS_ESTIMATION=shadow EXPERIMENTAL_VM_PLAYGROUND_FAST_VM_MODE=shadow EXPERIMENTAL_VM_PLAYGROUND_DB_PATH=/db/vm_playground EXPERIMENTAL_VM_PLAYGROUND_FIRST_PROCESSED_BATCH=123 @@ -64,6 +65,10 @@ mod tests { let config = ExperimentalVmConfig::from_env().unwrap(); assert_eq!(config.state_keeper_fast_vm_mode, FastVmMode::New); + assert_eq!( + config.api_fast_vm_mode_for_gas_estimation, + FastVmMode::Shadow + ); assert_eq!(config.playground.fast_vm_mode, FastVmMode::Shadow); assert_eq!(config.playground.db_path.unwrap(), "/db/vm_playground"); assert_eq!(config.playground.first_processed_batch, L1BatchNumber(123)); diff --git a/core/lib/protobuf_config/src/experimental.rs b/core/lib/protobuf_config/src/experimental.rs index 63fa0ca51eb5..e441e8e3d9a2 100644 --- a/core/lib/protobuf_config/src/experimental.rs +++ b/core/lib/protobuf_config/src/experimental.rs @@ -7,6 +7,14 @@ use zksync_protobuf::{repr::ProtoRepr, required}; use crate::{proto::experimental as proto, read_optional_repr}; +fn parse_vm_mode(raw: Option) -> anyhow::Result { + Ok(raw + .map(proto::FastVmMode::try_from) + .transpose() + .context("fast_vm_mode")? + .map_or_else(FastVmMode::default, |mode| mode.parse())) +} + impl ProtoRepr for proto::Db { type Type = configs::ExperimentalDBConfig; @@ -105,12 +113,10 @@ impl ProtoRepr for proto::Vm { fn read(&self) -> anyhow::Result { Ok(Self::Type { playground: read_optional_repr(&self.playground).unwrap_or_default(), - state_keeper_fast_vm_mode: self - .state_keeper_fast_vm_mode - .map(proto::FastVmMode::try_from) - .transpose() - .context("fast_vm_mode")? - .map_or_else(FastVmMode::default, |mode| mode.parse()), + state_keeper_fast_vm_mode: parse_vm_mode(self.state_keeper_fast_vm_mode)?, + api_fast_vm_mode_for_gas_estimation: parse_vm_mode( + self.api_fast_vm_mode_for_gas_estimation, + )?, }) } @@ -120,6 +126,9 @@ impl ProtoRepr for proto::Vm { state_keeper_fast_vm_mode: Some( proto::FastVmMode::new(this.state_keeper_fast_vm_mode).into(), ), + api_fast_vm_mode_for_gas_estimation: Some( + proto::FastVmMode::new(this.api_fast_vm_mode_for_gas_estimation).into(), + ), } } } diff --git a/core/lib/protobuf_config/src/proto/config/experimental.proto b/core/lib/protobuf_config/src/proto/config/experimental.proto index 5e1d045ca670..1eff89afb8c6 100644 --- a/core/lib/protobuf_config/src/proto/config/experimental.proto +++ b/core/lib/protobuf_config/src/proto/config/experimental.proto @@ -37,4 +37,5 @@ message VmPlayground { message Vm { optional VmPlayground playground = 1; // optional optional FastVmMode state_keeper_fast_vm_mode = 2; // optional; if not set, fast VM is not used + optional FastVmMode api_fast_vm_mode_for_gas_estimation = 3; // optional; if not set, fast VM is not used } diff --git a/core/node/api_server/src/execution_sandbox/execute.rs b/core/node/api_server/src/execution_sandbox/execute.rs index be529af7b072..17b393034559 100644 --- a/core/node/api_server/src/execution_sandbox/execute.rs +++ b/core/node/api_server/src/execution_sandbox/execute.rs @@ -109,6 +109,8 @@ impl SandboxExecutor { ) -> Self { let mut executor = MainOneshotExecutor::new(missed_storage_invocation_limit); executor.set_fast_vm_modes(options.fast_vm_modes); + #[cfg(test)] + executor.panic_on_divergence(); executor .set_execution_latency_histogram(&SANDBOX_METRICS.sandbox[&SandboxStage::Execution]); Self { diff --git a/core/node/node_framework/src/implementations/layers/web3_api/tx_sender.rs b/core/node/node_framework/src/implementations/layers/web3_api/tx_sender.rs index a09938055fae..519882bebb95 100644 --- a/core/node/node_framework/src/implementations/layers/web3_api/tx_sender.rs +++ b/core/node/node_framework/src/implementations/layers/web3_api/tx_sender.rs @@ -6,7 +6,8 @@ use zksync_node_api_server::{ tx_sender::{SandboxExecutorOptions, TxSenderBuilder, TxSenderConfig}, }; use zksync_state::{PostgresStorageCaches, PostgresStorageCachesTask}; -use zksync_types::{AccountTreeId, Address}; +use zksync_types::{vm::FastVmMode, AccountTreeId, Address}; +use zksync_vm_executor::oneshot::OneshotExecutorVmModes; use zksync_web3_decl::{ client::{DynClient, L2}, jsonrpsee, @@ -59,6 +60,7 @@ pub struct TxSenderLayer { postgres_storage_caches_config: PostgresStorageCachesConfig, max_vm_concurrency: usize, whitelisted_tokens_for_aa_cache: bool, + vm_modes: OneshotExecutorVmModes, } #[derive(Debug, FromContext)] @@ -94,6 +96,7 @@ impl TxSenderLayer { postgres_storage_caches_config, max_vm_concurrency, whitelisted_tokens_for_aa_cache: false, + vm_modes: OneshotExecutorVmModes::default(), } } @@ -105,6 +108,12 @@ impl TxSenderLayer { self.whitelisted_tokens_for_aa_cache = value; self } + + /// Sets the fast VM modes used for gas estimation. + pub fn with_gas_estimation_vm_mode(mut self, mode: FastVmMode) -> Self { + self.vm_modes.gas_estimation = mode; + self + } } #[async_trait::async_trait] @@ -147,12 +156,13 @@ impl WiringLayer for TxSenderLayer { // TODO (BFT-138): Allow to dynamically reload API contracts let config = self.tx_sender_config; - let executor_options = SandboxExecutorOptions::new( + let mut executor_options = SandboxExecutorOptions::new( config.chain_id, AccountTreeId::new(config.fee_account_addr), config.validation_computational_gas_limit, ) .await?; + executor_options.set_fast_vm_modes(self.vm_modes); // Build `TxSender`. let mut tx_sender = TxSenderBuilder::new(config, replica_pool, tx_sink); From e87471c52797730464b82d72a0f6db60bd44a283 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 8 Oct 2024 12:44:40 +0300 Subject: [PATCH 09/18] Fix `TxHasEnded` hook processing divergence --- .../versions/vm_fast/tests/l1_tx_execution.rs | 48 ++++++++++++++++++- core/lib/multivm/src/versions/vm_fast/vm.rs | 27 +++++++++-- .../vm_latest/tests/l1_tx_execution.rs | 44 ++++++++++++++++- 3 files changed, 112 insertions(+), 7 deletions(-) diff --git a/core/lib/multivm/src/versions/vm_fast/tests/l1_tx_execution.rs b/core/lib/multivm/src/versions/vm_fast/tests/l1_tx_execution.rs index 1abb1e39e19b..9fa6eddf5dcd 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/l1_tx_execution.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/l1_tx_execution.rs @@ -1,16 +1,21 @@ +use assert_matches::assert_matches; use ethabi::Token; use zksync_contracts::l1_messenger_contract; use zksync_system_constants::{BOOTLOADER_ADDRESS, L1_MESSENGER_ADDRESS}; use zksync_types::{ get_code_key, get_known_code_key, l2_to_l1_log::{L2ToL1Log, UserL2ToL1Log}, - Execute, ExecuteTransactionCommon, U256, + Address, Execute, ExecuteTransactionCommon, U256, }; use zksync_utils::{h256_to_u256, u256_to_h256}; use crate::{ - interface::{TxExecutionMode, VmExecutionMode, VmInterface, VmInterfaceExt}, + interface::{ + ExecutionResult, TxExecutionMode, VmExecutionMode, VmInterface, VmInterfaceExt, + VmRevertReason, + }, utils::StorageWritesDeduplicator, + versions::testonly::ContractToDeploy, vm_fast::{ tests::{ tester::{TxType, VmTesterBuilder}, @@ -197,3 +202,42 @@ fn test_l1_tx_execution_high_gas_limit() { assert!(res.result.is_failed(), "The transaction should've failed"); } + +#[test] +fn test_l1_tx_execution_gas_estimation_with_low_gas() { + let counter_contract = read_test_contract(); + let counter_address = Address::repeat_byte(0x11); + let mut vm = VmTesterBuilder::new() + .with_empty_in_memory_storage() + .with_base_system_smart_contracts(BASE_SYSTEM_CONTRACTS.clone()) + .with_execution_mode(TxExecutionMode::EstimateFee) + .with_custom_contracts(vec![ContractToDeploy::new( + counter_contract, + counter_address, + )]) + .with_random_rich_accounts(1) + .build(); + + let account = &mut vm.rich_accounts[0]; + let mut tx = account.get_test_contract_transaction( + counter_address, + false, + None, + false, + TxType::L1 { serial_id: 0 }, + ); + let ExecuteTransactionCommon::L1(data) = &mut tx.common_data else { + unreachable!(); + }; + // This gas limit is chosen so that transaction starts getting executed by the bootloader, but then runs out of gas + // before its execution result is posted. + data.gas_limit = 15_000.into(); + + vm.vm.push_transaction(tx); + let res = vm.vm.execute(VmExecutionMode::OneTx); + assert_matches!( + &res.result, + ExecutionResult::Revert { output: VmRevertReason::General { msg, .. } } + if msg.contains("reverted with empty reason") + ); +} diff --git a/core/lib/multivm/src/versions/vm_fast/vm.rs b/core/lib/multivm/src/versions/vm_fast/vm.rs index 10be6d88b045..54defedb3988 100644 --- a/core/lib/multivm/src/versions/vm_fast/vm.rs +++ b/core/lib/multivm/src/versions/vm_fast/vm.rs @@ -49,8 +49,8 @@ use crate::{ }, vm_latest::{ constants::{ - get_vm_hook_params_start_position, get_vm_hook_position, OPERATOR_REFUNDS_OFFSET, - TX_GAS_LIMIT_OFFSET, VM_HOOK_PARAMS_COUNT, + get_result_success_first_slot, get_vm_hook_params_start_position, get_vm_hook_position, + OPERATOR_REFUNDS_OFFSET, TX_GAS_LIMIT_OFFSET, VM_HOOK_PARAMS_COUNT, }, MultiVMSubversion, }, @@ -205,7 +205,22 @@ impl Vm { } Hook::TxHasEnded => { if let VmExecutionMode::OneTx = execution_mode { - break (last_tx_result.take().unwrap(), false); + // The bootloader may invoke `TxHasEnded` hook without posting a tx result previously. One case when this can happen + // is estimating gas for L1 transactions, if a transaction runs out of gas during execution. + let tx_result = last_tx_result.take().unwrap_or_else(|| { + let tx_has_failed = self.get_tx_result().is_zero(); + if tx_has_failed { + let output = VmRevertReason::General { + msg: "Transaction reverted with empty reason. Possibly out of gas" + .to_string(), + data: vec![], + }; + ExecutionResult::Revert { output } + } else { + ExecutionResult::Success { output: vec![] } + } + }); + break (tx_result, false); } } Hook::AskOperatorForRefund => { @@ -353,6 +368,12 @@ impl Vm { .unwrap() } + fn get_tx_result(&self) -> U256 { + let tx_idx = self.bootloader_state.current_tx(); + let slot = get_result_success_first_slot(VM_VERSION) as usize + tx_idx; + self.read_word_from_bootloader_heap(slot) + } + fn get_debug_log(&self) -> (String, String) { let hook_params = self.get_hook_params(); let mut msg = u256_to_h256(hook_params[0]).as_bytes().to_vec(); diff --git a/core/lib/multivm/src/versions/vm_latest/tests/l1_tx_execution.rs b/core/lib/multivm/src/versions/vm_latest/tests/l1_tx_execution.rs index 0fc12848227e..89490554846e 100644 --- a/core/lib/multivm/src/versions/vm_latest/tests/l1_tx_execution.rs +++ b/core/lib/multivm/src/versions/vm_latest/tests/l1_tx_execution.rs @@ -1,3 +1,4 @@ +use assert_matches::assert_matches; use ethabi::Token; use zksync_contracts::l1_messenger_contract; use zksync_system_constants::{BOOTLOADER_ADDRESS, L1_MESSENGER_ADDRESS}; @@ -5,12 +6,15 @@ use zksync_test_account::Account; use zksync_types::{ get_code_key, get_known_code_key, l2_to_l1_log::{L2ToL1Log, UserL2ToL1Log}, - Execute, ExecuteTransactionCommon, K256PrivateKey, U256, + Address, Execute, ExecuteTransactionCommon, K256PrivateKey, U256, }; use zksync_utils::u256_to_h256; use crate::{ - interface::{TxExecutionMode, VmExecutionMode, VmInterface, VmInterfaceExt}, + interface::{ + ExecutionResult, TxExecutionMode, VmExecutionMode, VmInterface, VmInterfaceExt, + VmRevertReason, + }, utils::StorageWritesDeduplicator, vm_latest::{ tests::{ @@ -194,3 +198,39 @@ fn test_l1_tx_execution_high_gas_limit() { assert!(res.result.is_failed(), "The transaction should've failed"); } + +#[test] +fn test_l1_tx_execution_gas_estimation_with_low_gas() { + let counter_contract = read_test_contract(); + let counter_address = Address::repeat_byte(0x11); + let mut vm = VmTesterBuilder::new(HistoryEnabled) + .with_empty_in_memory_storage() + .with_base_system_smart_contracts(BASE_SYSTEM_CONTRACTS.clone()) + .with_execution_mode(TxExecutionMode::EstimateFee) + .with_custom_contracts(vec![(counter_contract, counter_address, false)]) + .with_random_rich_accounts(1) + .build(); + + let account = &mut vm.rich_accounts[0]; + let mut tx = account.get_test_contract_transaction( + counter_address, + false, + None, + false, + TxType::L1 { serial_id: 0 }, + ); + let ExecuteTransactionCommon::L1(data) = &mut tx.common_data else { + unreachable!(); + }; + // This gas limit is chosen so that transaction starts getting executed by the bootloader, but then runs out of gas + // before its execution result is posted. + data.gas_limit = 15_000.into(); + + vm.vm.push_transaction(tx); + let res = vm.vm.execute(VmExecutionMode::OneTx); + assert_matches!( + &res.result, + ExecutionResult::Revert { output: VmRevertReason::General { msg, .. } } + if msg.contains("reverted with empty reason") + ); +} From 2a230ba2144af52461986e804e47f98631b82891 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 8 Oct 2024 12:49:46 +0300 Subject: [PATCH 10/18] Test estimating gas for L1 transaction --- core/node/api_server/src/testonly.rs | 29 +++++++++++++-- .../src/tx_sender/tests/gas_estimation.rs | 37 +++++++++++++++++-- 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/core/node/api_server/src/testonly.rs b/core/node/api_server/src/testonly.rs index 8dc7915385a1..5a56759abb7b 100644 --- a/core/node/api_server/src/testonly.rs +++ b/core/node/api_server/src/testonly.rs @@ -10,7 +10,7 @@ use zksync_contracts::{ use zksync_dal::{Connection, Core, CoreDal}; use zksync_multivm::utils::derive_base_fee_and_gas_per_pubdata; use zksync_node_fee_model::BatchFeeModelInputProvider; -use zksync_system_constants::L2_BASE_TOKEN_ADDRESS; +use zksync_system_constants::{L2_BASE_TOKEN_ADDRESS, REQUIRED_L1_TO_L2_GAS_PER_PUBDATA_BYTE}; use zksync_types::{ api::state_override::{Bytecode, OverrideAccount, OverrideState, StateOverride}, ethabi, @@ -18,11 +18,12 @@ use zksync_types::{ fee::Fee, fee_model::FeeParams, get_code_key, get_known_code_key, + l1::L1Tx, l2::L2Tx, - transaction_request::{CallRequest, PaymasterParams}, + transaction_request::{CallRequest, Eip712Meta, PaymasterParams}, utils::storage_key_for_eth_balance, AccountTreeId, Address, K256PrivateKey, L2BlockNumber, L2ChainId, Nonce, ProtocolVersionId, - StorageKey, StorageLog, H256, U256, + StorageKey, StorageLog, EIP_712_TX_TYPE, H256, U256, }; use zksync_utils::{address_to_u256, u256_to_h256}; @@ -323,6 +324,8 @@ pub(crate) trait TestAccount { fn create_counter_tx(&self, increment: U256, revert: bool) -> L2Tx; + fn create_l1_counter_tx(&self, increment: U256, revert: bool) -> L1Tx; + fn query_counter_value(&self) -> CallRequest; fn create_infinite_loop_tx(&self) -> L2Tx; @@ -462,6 +465,26 @@ impl TestAccount for K256PrivateKey { .unwrap() } + fn create_l1_counter_tx(&self, increment: U256, revert: bool) -> L1Tx { + let calldata = load_contract(COUNTER_CONTRACT_PATH) + .function("incrementWithRevert") + .expect("no `incrementWithRevert` function") + .encode_input(&[Token::Uint(increment), Token::Bool(revert)]) + .expect("failed encoding `incrementWithRevert` input"); + let request = CallRequest { + data: Some(calldata.into()), + from: Some(self.address()), + to: Some(StateBuilder::COUNTER_CONTRACT_ADDRESS), + transaction_type: Some(EIP_712_TX_TYPE.into()), + eip712_meta: Some(Eip712Meta { + gas_per_pubdata: REQUIRED_L1_TO_L2_GAS_PER_PUBDATA_BYTE.into(), + ..Eip712Meta::default() + }), + ..CallRequest::default() + }; + request.try_into().unwrap() + } + fn query_counter_value(&self) -> CallRequest { let calldata = load_contract(COUNTER_CONTRACT_PATH) .function("get") diff --git a/core/node/api_server/src/tx_sender/tests/gas_estimation.rs b/core/node/api_server/src/tx_sender/tests/gas_estimation.rs index 086313a8562d..ed4c04f3a757 100644 --- a/core/node/api_server/src/tx_sender/tests/gas_estimation.rs +++ b/core/node/api_server/src/tx_sender/tests/gas_estimation.rs @@ -73,6 +73,27 @@ async fn initial_estimate_for_load_test_transaction(tx_params: LoadnextContractE test_initial_estimate(state_override, tx, DEFAULT_MULTIPLIER).await; } +#[tokio::test] +async fn initial_gas_estimate_for_l1_transaction() { + let alice = K256PrivateKey::random(); + let state_override = StateBuilder::default().with_counter_contract(0).build(); + let tx = alice.create_l1_counter_tx(1.into(), false); + + let pool = ConnectionPool::::constrained_test_pool(1).await; + let tx_sender = create_real_tx_sender(pool).await; + let mut estimator = GasEstimator::new(&tx_sender, tx.into(), Some(state_override)) + .await + .unwrap(); + estimator.adjust_transaction_fee(); + let initial_estimate = estimator.initialize().await.unwrap(); + assert!(initial_estimate.total_gas_charged.is_none()); + + let (vm_result, _) = estimator.unadjusted_step(15_000).await.unwrap(); + assert!(vm_result.result.is_failed(), "{:?}", vm_result.result); + let (vm_result, _) = estimator.unadjusted_step(1_000_000).await.unwrap(); + assert!(!vm_result.result.is_failed(), "{:?}", vm_result.result); +} + #[test_casing(2, [false, true])] #[tokio::test] async fn initial_estimate_for_deep_recursion(with_reads: bool) { @@ -312,16 +333,17 @@ async fn insufficient_funds_error_for_transfer() { async fn test_estimating_gas( state_override: StateOverride, - tx: L2Tx, + tx: impl Into, acceptable_overestimation: u64, ) { + let tx = tx.into(); let pool = ConnectionPool::::constrained_test_pool(1).await; let tx_sender = create_real_tx_sender(pool).await; let fee_scale_factor = 1.0; let fee = tx_sender .get_txs_fee_in_wei( - tx.clone().into(), + tx.clone(), fee_scale_factor, acceptable_overestimation, Some(state_override.clone()), @@ -338,7 +360,7 @@ async fn test_estimating_gas( let fee = tx_sender .get_txs_fee_in_wei( - tx.into(), + tx, fee_scale_factor, acceptable_overestimation, Some(state_override.clone()), @@ -370,6 +392,15 @@ async fn estimating_gas_for_transfer(acceptable_overestimation: u64) { test_estimating_gas(state_override, tx, acceptable_overestimation).await; } +#[tokio::test] +async fn estimating_gas_for_l1_transaction() { + let alice = K256PrivateKey::random(); + let state_override = StateBuilder::default().with_counter_contract(0).build(); + let tx = alice.create_l1_counter_tx(1.into(), false); + + test_estimating_gas(state_override, tx, 0).await; +} + #[test_casing(10, Product((LOAD_TEST_CASES, [0, 100])))] #[tokio::test] async fn estimating_gas_for_load_test_tx( From 12fdfbc28251d76e1fc974106abfd9e07d95a0ac Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 8 Oct 2024 12:58:29 +0300 Subject: [PATCH 11/18] Enable fast VM for gas estimation in CI load test --- etc/env/file_based/overrides/tests/loadtest-new.yaml | 1 + etc/env/file_based/overrides/tests/loadtest-old.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/etc/env/file_based/overrides/tests/loadtest-new.yaml b/etc/env/file_based/overrides/tests/loadtest-new.yaml index 2167f7347e09..e0d40a90391b 100644 --- a/etc/env/file_based/overrides/tests/loadtest-new.yaml +++ b/etc/env/file_based/overrides/tests/loadtest-new.yaml @@ -3,5 +3,6 @@ db: mode: LIGHTWEIGHT experimental_vm: state_keeper_fast_vm_mode: NEW + api_fast_vm_mode_for_gas_estimation: NEW mempool: delay_interval: 50 diff --git a/etc/env/file_based/overrides/tests/loadtest-old.yaml b/etc/env/file_based/overrides/tests/loadtest-old.yaml index a2d66d1cf4a7..f4336199842e 100644 --- a/etc/env/file_based/overrides/tests/loadtest-old.yaml +++ b/etc/env/file_based/overrides/tests/loadtest-old.yaml @@ -3,5 +3,6 @@ db: mode: LIGHTWEIGHT experimental_vm: state_keeper_fast_vm_mode: OLD + api_fast_vm_mode_for_gas_estimation: OLD mempool: delay_interval: 50 From b4a59c9f0675ab261b9362b65f1d3d7aa9b5551c Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 8 Oct 2024 16:59:09 +0300 Subject: [PATCH 12/18] Refactor `VmSandbox` --- core/lib/vm_executor/src/oneshot/mod.rs | 180 +++++++++++------------- 1 file changed, 85 insertions(+), 95 deletions(-) diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs index a48f2dfcd610..273ac8b60542 100644 --- a/core/lib/vm_executor/src/oneshot/mod.rs +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -17,7 +17,7 @@ use once_cell::sync::OnceCell; use zksync_multivm::{ interface::{ executor::{OneshotExecutor, TransactionValidator}, - storage::{ReadStorage, StoragePtr, StorageView, StorageWithOverrides}, + storage::{ReadStorage, StorageView, StorageWithOverrides}, tracer::{ValidationError, ValidationParams}, utils::{DivergenceHandler, ShadowVm}, Call, ExecutionResult, OneshotEnv, OneshotTracingParams, OneshotTransactionExecutionResult, @@ -63,7 +63,7 @@ pub struct OneshotExecutorVmModes { pub gas_estimation: FastVmMode, /// Mode used for `eth_call`s. pub call: FastVmMode, - /// Mode used for transaction execution (doesn't include the validation step). + /// Mode used for transaction execution. This temporarily doesn't include the validation step since it's not supported by the fast VM. pub tx_execution: FastVmMode, } @@ -170,20 +170,17 @@ where self.missed_storage_invocation_limit } }; - let execution_latency_histogram = self.execution_latency_histogram; - let fast_vm_mode = self.select_fast_vm_mode(&env, &tracing_params); - let panic_on_divergence = self.panic_on_divergence; + let sandbox = VmSandbox { + fast_vm_mode: self.select_fast_vm_mode(&env, &tracing_params), + panic_on_divergence: self.panic_on_divergence, + storage, + env, + execution_args: args, + execution_latency_histogram: self.execution_latency_histogram, + }; tokio::task::spawn_blocking(move || { - let executor = VmSandbox::new( - fast_vm_mode, - panic_on_divergence, - storage, - env, - args, - execution_latency_histogram, - ); - executor.apply(|vm, transaction| { + sandbox.execute_in_vm(|vm, transaction| { vm.inspect_transaction_with_bytecode_compression( missed_storage_invocation_limit, tracing_params, @@ -214,25 +211,25 @@ where "Unexpected execution mode for tx validation: {:?} (expected `VerifyExecute`)", env.system.execution_mode ); - let execution_latency_histogram = self.execution_latency_histogram; + + let sandbox = VmSandbox { + fast_vm_mode: FastVmMode::Old, + panic_on_divergence: self.panic_on_divergence, + storage, + env, + execution_args: TxExecutionArgs::for_validation(tx), + execution_latency_histogram: self.execution_latency_histogram, + }; tokio::task::spawn_blocking(move || { let (validation_tracer, mut validation_result) = ValidationTracer::::new( validation_params, - env.system.version.into(), + sandbox.env.system.version.into(), ); let tracers = vec![validation_tracer.into_tracer_pointer()]; - let executor = VmSandbox::new( - FastVmMode::Old, - false, - storage, - env, - TxExecutionArgs::for_validation(tx), - execution_latency_histogram, - ); - let exec_result = executor.apply(|vm, transaction| { + let exec_result = sandbox.execute_in_vm(|vm, transaction| { let Vm::Legacy(vm) = vm else { unreachable!("Fast VM is never used for validation yet"); }; @@ -316,71 +313,18 @@ impl Vm { } } +/// Full parameters necessary to instantiate a VM for oneshot execution. #[derive(Debug)] -struct VmSandbox { - vm: Vm>, - storage_view: StoragePtr>>, - transaction: Transaction, +struct VmSandbox { + fast_vm_mode: FastVmMode, + panic_on_divergence: bool, + storage: StorageWithOverrides, + env: OneshotEnv, + execution_args: TxExecutionArgs, execution_latency_histogram: Option<&'static vise::Histogram>, } impl VmSandbox { - /// This method is blocking. - fn new( - fast_vm_mode: FastVmMode, - panic_on_divergence: bool, - mut storage: StorageWithOverrides, - mut env: OneshotEnv, - execution_args: TxExecutionArgs, - execution_latency_histogram: Option<&'static vise::Histogram>, - ) -> Self { - Self::setup_storage(&mut storage, &execution_args, env.current_block); - - let mode = env.system.execution_mode; - let protocol_version = env.system.version; - if execution_args.adjust_pubdata_price { - env.l1_batch.fee_input = adjust_pubdata_price_for_tx( - env.l1_batch.fee_input, - execution_args.transaction.gas_per_pubdata_byte_limit(), - env.l1_batch.enforced_base_fee.map(U256::from), - protocol_version.into(), - ); - }; - - let storage_view = StorageView::new(storage).to_rc_ptr(); - let vm = match fast_vm_mode { - FastVmMode::Old => Vm::Legacy(LegacyVmInstance::new_with_specific_version( - env.l1_batch, - env.system, - storage_view.clone(), - protocol_version.into_api_vm_version(), - )), - FastVmMode::New => Vm::Fast(FastVmInstance::fast( - env.l1_batch, - env.system, - storage_view.clone(), - )), - FastVmMode::Shadow => { - let mut vm = ShadowVm::new(env.l1_batch, env.system, storage_view.clone()); - if !panic_on_divergence { - let transaction = format!("{:?}", execution_args.transaction); - let handler = DivergenceHandler::new(move |errors, _| { - tracing::error!(transaction, ?mode, "{errors}"); - }); - vm.set_divergence_handler(handler); - } - Vm::Fast(FastVmInstance::Shadowed(vm)) - } - }; - - Self { - vm, - storage_view, - transaction: execution_args.transaction, - execution_latency_histogram, - } - } - /// This method is blocking. fn setup_storage( storage: &mut StorageWithOverrides, @@ -429,32 +373,78 @@ impl VmSandbox { } } - pub(super) fn apply(mut self, apply_fn: F) -> T - where - F: FnOnce(&mut Vm>, Transaction) -> T, - { + /// This method is blocking. + fn execute_in_vm( + mut self, + action: impl FnOnce(&mut Vm>, Transaction) -> T, + ) -> T { + Self::setup_storage( + &mut self.storage, + &self.execution_args, + self.env.current_block, + ); + + let protocol_version = self.env.system.version; + let mode = self.env.system.execution_mode; + if self.execution_args.adjust_pubdata_price { + self.env.l1_batch.fee_input = adjust_pubdata_price_for_tx( + self.env.l1_batch.fee_input, + self.execution_args.transaction.gas_per_pubdata_byte_limit(), + self.env.l1_batch.enforced_base_fee.map(U256::from), + protocol_version.into(), + ); + }; + + let transaction = self.execution_args.transaction; let tx_id = format!( "{:?}-{}", - self.transaction.initiator_account(), - self.transaction.nonce().unwrap_or(Nonce(0)) + transaction.initiator_account(), + transaction.nonce().unwrap_or(Nonce(0)) ); + let storage_view = StorageView::new(self.storage).to_rc_ptr(); + let mut vm = match self.fast_vm_mode { + FastVmMode::Old => Vm::Legacy(LegacyVmInstance::new_with_specific_version( + self.env.l1_batch, + self.env.system, + storage_view.clone(), + protocol_version.into_api_vm_version(), + )), + FastVmMode::New => Vm::Fast(FastVmInstance::fast( + self.env.l1_batch, + self.env.system, + storage_view.clone(), + )), + FastVmMode::Shadow => { + let mut vm = + ShadowVm::new(self.env.l1_batch, self.env.system, storage_view.clone()); + if !self.panic_on_divergence { + let transaction = format!("{:?}", transaction); + let handler = DivergenceHandler::new(move |errors, _| { + tracing::error!(transaction, ?mode, "{errors}"); + }); + vm.set_divergence_handler(handler); + } + Vm::Fast(FastVmInstance::Shadowed(vm)) + } + }; + let started_at = Instant::now(); - let result = apply_fn(&mut self.vm, self.transaction); + let result = action(&mut vm, transaction); let vm_execution_took = started_at.elapsed(); if let Some(histogram) = self.execution_latency_histogram { histogram.observe(vm_execution_took); } - match &self.vm { + match &vm { Vm::Legacy(vm) => { let memory_metrics = vm.record_vm_memory_metrics(); metrics::report_vm_memory_metrics( &tx_id, &memory_metrics, vm_execution_took, - &self.storage_view.borrow().stats(), + &storage_view.borrow().stats(), ); } Vm::Fast(_) => { @@ -463,7 +453,7 @@ impl VmSandbox { metrics::report_vm_storage_metrics( &format!("Tx {tx_id}"), vm_execution_took, - &self.storage_view.borrow().stats(), + &storage_view.borrow().stats(), ); } } From 35dcd5beea193f8e94ff0042f0a284e50006404c Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 8 Oct 2024 17:02:24 +0300 Subject: [PATCH 13/18] Optimize gas estimation for "new" load test --- etc/env/file_based/overrides/tests/loadtest-new.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/etc/env/file_based/overrides/tests/loadtest-new.yaml b/etc/env/file_based/overrides/tests/loadtest-new.yaml index e0d40a90391b..3d51cbb322da 100644 --- a/etc/env/file_based/overrides/tests/loadtest-new.yaml +++ b/etc/env/file_based/overrides/tests/loadtest-new.yaml @@ -1,6 +1,9 @@ db: merkle_tree: mode: LIGHTWEIGHT +api: + web3_json_rpc: + estimate_gas_optimize_search: true experimental_vm: state_keeper_fast_vm_mode: NEW api_fast_vm_mode_for_gas_estimation: NEW From 47979e810e2b5d0b1479067133785ed71bdaa6ee Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 14 Oct 2024 17:49:48 +0300 Subject: [PATCH 14/18] Deduplicate test code --- core/lib/vm_executor/src/oneshot/tests.rs | 76 ++--------------------- core/lib/vm_executor/src/testonly.rs | 32 +++++++++- 2 files changed, 35 insertions(+), 73 deletions(-) diff --git a/core/lib/vm_executor/src/oneshot/tests.rs b/core/lib/vm_executor/src/oneshot/tests.rs index 7c72638f0b62..e2ad9e1c72d7 100644 --- a/core/lib/vm_executor/src/oneshot/tests.rs +++ b/core/lib/vm_executor/src/oneshot/tests.rs @@ -1,87 +1,21 @@ //! Oneshot executor tests. use assert_matches::assert_matches; -use once_cell::sync::Lazy; use test_casing::{test_casing, Product}; -use zksync_contracts::BaseSystemContracts; -use zksync_multivm::{ - interface::{storage::InMemoryStorage, L1BatchEnv, L2BlockEnv, SystemEnv}, - utils::derive_base_fee_and_gas_per_pubdata, - vm_latest::constants::BATCH_COMPUTATIONAL_GAS_LIMIT, - zk_evm_latest::ethereum_types::Address, -}; -use zksync_types::{ - block::L2BlockHasher, fee::Fee, fee_model::BatchFeeInput, transaction_request::PaymasterParams, - K256PrivateKey, L1BatchNumber, L2BlockNumber, L2ChainId, ProtocolVersionId, H256, - ZKPORTER_IS_AVAILABLE, -}; +use zksync_multivm::interface::storage::InMemoryStorage; +use zksync_types::{ProtocolVersionId, H256}; use zksync_utils::bytecode::hash_bytecode; use super::*; - -static BASE_SYSTEM_CONTRACTS: Lazy = - Lazy::new(BaseSystemContracts::load_from_disk); +use crate::testonly::{ + create_l2_transaction, default_l1_batch_env, default_system_env, FAST_VM_MODES, +}; const EXEC_MODES: [TxExecutionMode; 3] = [ TxExecutionMode::EstimateFee, TxExecutionMode::EthCall, TxExecutionMode::VerifyExecute, ]; -const FAST_VM_MODES: [FastVmMode; 3] = [FastVmMode::Old, FastVmMode::New, FastVmMode::Shadow]; - -fn default_system_env(execution_mode: TxExecutionMode) -> SystemEnv { - SystemEnv { - zk_porter_available: ZKPORTER_IS_AVAILABLE, - version: ProtocolVersionId::latest(), - base_system_smart_contracts: BASE_SYSTEM_CONTRACTS.clone(), - bootloader_gas_limit: BATCH_COMPUTATIONAL_GAS_LIMIT, - execution_mode, - default_validation_computational_gas_limit: BATCH_COMPUTATIONAL_GAS_LIMIT, - chain_id: L2ChainId::default(), - } -} - -fn default_l1_batch_env(number: u32) -> L1BatchEnv { - L1BatchEnv { - previous_batch_hash: Some(H256::zero()), - number: L1BatchNumber(number), - timestamp: number.into(), - fee_account: Address::repeat_byte(0x22), - enforced_base_fee: None, - first_l2_block: L2BlockEnv { - number, - timestamp: number.into(), - prev_block_hash: L2BlockHasher::legacy_hash(L2BlockNumber(number - 1)), - max_virtual_blocks_to_create: 1, - }, - fee_input: BatchFeeInput::sensible_l1_pegged_default(), - } -} - -fn create_l2_transaction(value: U256, nonce: Nonce) -> L2Tx { - let (max_fee_per_gas, gas_per_pubdata_limit) = derive_base_fee_and_gas_per_pubdata( - BatchFeeInput::sensible_l1_pegged_default(), - ProtocolVersionId::latest().into(), - ); - let fee = Fee { - gas_limit: 10_000_000.into(), - max_fee_per_gas: max_fee_per_gas.into(), - max_priority_fee_per_gas: 0_u64.into(), - gas_per_pubdata_limit: gas_per_pubdata_limit.into(), - }; - L2Tx::new_signed( - Some(Address::random()), - vec![], - nonce, - fee, - value, - L2ChainId::default(), - &K256PrivateKey::random(), - vec![], - PaymasterParams::default(), - ) - .unwrap() -} #[test] fn selecting_vm_for_execution() { diff --git a/core/lib/vm_executor/src/testonly.rs b/core/lib/vm_executor/src/testonly.rs index 5bcd604a4324..2fa7f075db71 100644 --- a/core/lib/vm_executor/src/testonly.rs +++ b/core/lib/vm_executor/src/testonly.rs @@ -2,11 +2,14 @@ use once_cell::sync::Lazy; use zksync_contracts::BaseSystemContracts; use zksync_multivm::{ interface::{L1BatchEnv, L2BlockEnv, SystemEnv, TxExecutionMode}, + utils::derive_base_fee_and_gas_per_pubdata, vm_latest::constants::BATCH_COMPUTATIONAL_GAS_LIMIT, + zk_evm_latest::ethereum_types::U256, }; use zksync_types::{ - block::L2BlockHasher, fee_model::BatchFeeInput, vm::FastVmMode, Address, L1BatchNumber, - L2BlockNumber, L2ChainId, ProtocolVersionId, H256, ZKPORTER_IS_AVAILABLE, + block::L2BlockHasher, fee::Fee, fee_model::BatchFeeInput, l2::L2Tx, + transaction_request::PaymasterParams, vm::FastVmMode, Address, K256PrivateKey, L1BatchNumber, + L2BlockNumber, L2ChainId, Nonce, ProtocolVersionId, H256, ZKPORTER_IS_AVAILABLE, }; static BASE_SYSTEM_CONTRACTS: Lazy = @@ -43,3 +46,28 @@ pub(crate) fn default_l1_batch_env(number: u32) -> L1BatchEnv { fee_input: BatchFeeInput::sensible_l1_pegged_default(), } } + +pub(crate) fn create_l2_transaction(value: U256, nonce: Nonce) -> L2Tx { + let (max_fee_per_gas, gas_per_pubdata_limit) = derive_base_fee_and_gas_per_pubdata( + BatchFeeInput::sensible_l1_pegged_default(), + ProtocolVersionId::latest().into(), + ); + let fee = Fee { + gas_limit: 10_000_000.into(), + max_fee_per_gas: max_fee_per_gas.into(), + max_priority_fee_per_gas: 0_u64.into(), + gas_per_pubdata_limit: gas_per_pubdata_limit.into(), + }; + L2Tx::new_signed( + Some(Address::random()), + vec![], + nonce, + fee, + value, + L2ChainId::default(), + &K256PrivateKey::random(), + vec![], + PaymasterParams::default(), + ) + .unwrap() +} From f312f5f006a5b6c860207db91fe71528b9a57b84 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 18 Oct 2024 09:41:48 +0300 Subject: [PATCH 15/18] Override config for integration tests --- .github/workflows/ci-core-reusable.yml | 8 ++++++-- etc/env/file_based/overrides/tests/integration.yaml | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 etc/env/file_based/overrides/tests/integration.yaml diff --git a/.github/workflows/ci-core-reusable.yml b/.github/workflows/ci-core-reusable.yml index 9aaa476d740d..8ca1c0b69b28 100644 --- a/.github/workflows/ci-core-reusable.yml +++ b/.github/workflows/ci-core-reusable.yml @@ -356,12 +356,16 @@ jobs: - name: Run servers run: | + # Override config for part of chains to test the default config as well + ci_run zkstack dev config-writer --path etc/env/file_based/overrides/tests/integration.yaml --chain era + ci_run zkstack dev config-writer --path etc/env/file_based/overrides/tests/integration.yaml --chain validium + ci_run zkstack server --ignore-prerequisites --chain era &> ${{ env.SERVER_LOGS_DIR }}/rollup.log & ci_run zkstack server --ignore-prerequisites --chain validium &> ${{ env.SERVER_LOGS_DIR }}/validium.log & ci_run zkstack server --ignore-prerequisites --chain custom_token &> ${{ env.SERVER_LOGS_DIR }}/custom_token.log & ci_run zkstack server --ignore-prerequisites --chain consensus \ - --components=api,tree,eth,state_keeper,housekeeper,commitment_generator,vm_runner_protective_reads,vm_runner_bwip,vm_playground,da_dispatcher,consensus \ - &> ${{ env.SERVER_LOGS_DIR }}/consensus.log & + --components=api,tree,eth,state_keeper,housekeeper,commitment_generator,vm_runner_protective_reads,vm_runner_bwip,vm_playground,da_dispatcher,consensus \ + &> ${{ env.SERVER_LOGS_DIR }}/consensus.log & ci_run sleep 5 diff --git a/etc/env/file_based/overrides/tests/integration.yaml b/etc/env/file_based/overrides/tests/integration.yaml new file mode 100644 index 000000000000..52bd4a3a5c95 --- /dev/null +++ b/etc/env/file_based/overrides/tests/integration.yaml @@ -0,0 +1,4 @@ +experimental_vm: + # Use the shadow VM mode everywhere to catch divergences as early as possible + state_keeper_fast_vm_mode: SHADOW + api_fast_vm_mode_for_gas_estimation: SHADOW From bcef33101b483db463c4ab7eda41f11fbbdf63b6 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 21 Oct 2024 13:24:05 +0300 Subject: [PATCH 16/18] Use single VM mode for oneshot executor ops --- core/bin/zksync_server/src/node_builder.rs | 3 +- core/lib/config/src/configs/experimental.rs | 5 +- core/lib/config/src/testonly.rs | 2 +- core/lib/env_config/src/vm_runner.rs | 5 +- core/lib/protobuf_config/src/experimental.rs | 8 +-- .../src/proto/config/experimental.proto | 2 +- core/lib/vm_executor/src/oneshot/mod.rs | 54 +++---------------- core/lib/vm_executor/src/oneshot/tests.rs | 23 +------- .../src/execution_sandbox/execute.rs | 2 +- core/node/api_server/src/tx_sender/mod.rs | 11 ++-- .../api_server/src/tx_sender/tests/mod.rs | 2 +- core/node/api_server/src/web3/tests/vm.rs | 2 +- .../layers/web3_api/tx_sender.rs | 13 +++-- 13 files changed, 33 insertions(+), 99 deletions(-) diff --git a/core/bin/zksync_server/src/node_builder.rs b/core/bin/zksync_server/src/node_builder.rs index d5b7ea827630..8f3b2158e3e3 100644 --- a/core/bin/zksync_server/src/node_builder.rs +++ b/core/bin/zksync_server/src/node_builder.rs @@ -324,8 +324,7 @@ impl MainNodeBuilder { postgres_storage_caches_config, rpc_config.vm_concurrency_limit(), ); - let layer = - layer.with_gas_estimation_vm_mode(vm_config.api_fast_vm_mode_for_gas_estimation); + let layer = layer.with_vm_mode(vm_config.api_fast_vm_mode); self.node.add_layer(layer); Ok(self) } diff --git a/core/lib/config/src/configs/experimental.rs b/core/lib/config/src/configs/experimental.rs index 030b278581e6..a87a221ef222 100644 --- a/core/lib/config/src/configs/experimental.rs +++ b/core/lib/config/src/configs/experimental.rs @@ -107,7 +107,8 @@ pub struct ExperimentalVmConfig { #[serde(default)] pub state_keeper_fast_vm_mode: FastVmMode, - /// Fast VM mode to use for gas estimation in the API server. + /// Fast VM mode to use in the API server. Currently, some operations are not supported by the fast VM (e.g., `debug_traceCall` + /// or transaction validation), so the legacy VM will always be used for them. #[serde(default)] - pub api_fast_vm_mode_for_gas_estimation: FastVmMode, + pub api_fast_vm_mode: FastVmMode, } diff --git a/core/lib/config/src/testonly.rs b/core/lib/config/src/testonly.rs index 3a16f065bd8a..34ee3ea36119 100644 --- a/core/lib/config/src/testonly.rs +++ b/core/lib/config/src/testonly.rs @@ -328,7 +328,7 @@ impl Distribution for EncodeDist { configs::ExperimentalVmConfig { playground: self.sample(rng), state_keeper_fast_vm_mode: gen_fast_vm_mode(rng), - api_fast_vm_mode_for_gas_estimation: gen_fast_vm_mode(rng), + api_fast_vm_mode: gen_fast_vm_mode(rng), } } } diff --git a/core/lib/env_config/src/vm_runner.rs b/core/lib/env_config/src/vm_runner.rs index 60fafcc3d3a6..944dd0f98e9b 100644 --- a/core/lib/env_config/src/vm_runner.rs +++ b/core/lib/env_config/src/vm_runner.rs @@ -65,10 +65,7 @@ mod tests { let config = ExperimentalVmConfig::from_env().unwrap(); assert_eq!(config.state_keeper_fast_vm_mode, FastVmMode::New); - assert_eq!( - config.api_fast_vm_mode_for_gas_estimation, - FastVmMode::Shadow - ); + assert_eq!(config.api_fast_vm_mode, FastVmMode::Shadow); assert_eq!(config.playground.fast_vm_mode, FastVmMode::Shadow); assert_eq!(config.playground.db_path.unwrap(), "/db/vm_playground"); assert_eq!(config.playground.first_processed_batch, L1BatchNumber(123)); diff --git a/core/lib/protobuf_config/src/experimental.rs b/core/lib/protobuf_config/src/experimental.rs index e441e8e3d9a2..750dc7b04f01 100644 --- a/core/lib/protobuf_config/src/experimental.rs +++ b/core/lib/protobuf_config/src/experimental.rs @@ -114,9 +114,7 @@ impl ProtoRepr for proto::Vm { Ok(Self::Type { playground: read_optional_repr(&self.playground).unwrap_or_default(), state_keeper_fast_vm_mode: parse_vm_mode(self.state_keeper_fast_vm_mode)?, - api_fast_vm_mode_for_gas_estimation: parse_vm_mode( - self.api_fast_vm_mode_for_gas_estimation, - )?, + api_fast_vm_mode: parse_vm_mode(self.api_fast_vm_mode)?, }) } @@ -126,9 +124,7 @@ impl ProtoRepr for proto::Vm { state_keeper_fast_vm_mode: Some( proto::FastVmMode::new(this.state_keeper_fast_vm_mode).into(), ), - api_fast_vm_mode_for_gas_estimation: Some( - proto::FastVmMode::new(this.api_fast_vm_mode_for_gas_estimation).into(), - ), + api_fast_vm_mode: Some(proto::FastVmMode::new(this.api_fast_vm_mode).into()), } } } diff --git a/core/lib/protobuf_config/src/proto/config/experimental.proto b/core/lib/protobuf_config/src/proto/config/experimental.proto index 1eff89afb8c6..87af8d3835c6 100644 --- a/core/lib/protobuf_config/src/proto/config/experimental.proto +++ b/core/lib/protobuf_config/src/proto/config/experimental.proto @@ -37,5 +37,5 @@ message VmPlayground { message Vm { optional VmPlayground playground = 1; // optional optional FastVmMode state_keeper_fast_vm_mode = 2; // optional; if not set, fast VM is not used - optional FastVmMode api_fast_vm_mode_for_gas_estimation = 3; // optional; if not set, fast VM is not used + optional FastVmMode api_fast_vm_mode = 3; // optional; if not set, fast VM is not used } diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs index 3fa0ebb3c8e9..a0fa171bdc6f 100644 --- a/core/lib/vm_executor/src/oneshot/mod.rs +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -60,48 +60,10 @@ mod mock; #[cfg(test)] mod tests; -/// Fast VM modes utilized for different kinds of operations. -#[derive(Debug, Clone, Copy, Default)] -pub struct OneshotExecutorVmModes { - /// Mode used for gas estimation. - pub gas_estimation: FastVmMode, - /// Mode used for `eth_call`s. - pub call: FastVmMode, - /// Mode used for transaction execution. This temporarily doesn't include the validation step since it's not supported by the fast VM. - pub tx_execution: FastVmMode, -} - -/// Sets all modes to the specified value. -impl From for OneshotExecutorVmModes { - fn from(mode: FastVmMode) -> Self { - Self { - gas_estimation: mode, - call: mode, - tx_execution: mode, - } - } -} - -impl OneshotExecutorVmModes { - fn uses_fast_vm(&self) -> bool { - !matches!(self.gas_estimation, FastVmMode::Old) - || !matches!(self.call, FastVmMode::Old) - || !matches!(self.tx_execution, FastVmMode::Old) - } - - fn get(&self, mode: TxExecutionMode) -> FastVmMode { - match mode { - TxExecutionMode::VerifyExecute => self.tx_execution, - TxExecutionMode::EthCall => self.call, - TxExecutionMode::EstimateFee => self.gas_estimation, - } - } -} - /// Main [`OneshotExecutor`] implementation used by the API server. #[derive(Debug)] pub struct MainOneshotExecutor { - fast_vm_modes: OneshotExecutorVmModes, + fast_vm_mode: FastVmMode, panic_on_divergence: bool, missed_storage_invocation_limit: usize, execution_latency_histogram: Option<&'static vise::Histogram>, @@ -112,21 +74,21 @@ impl MainOneshotExecutor { /// The limit is applied for calls and gas estimations, but not during transaction validation. pub fn new(missed_storage_invocation_limit: usize) -> Self { Self { - fast_vm_modes: OneshotExecutorVmModes::default(), + fast_vm_mode: FastVmMode::Old, panic_on_divergence: false, missed_storage_invocation_limit, execution_latency_histogram: None, } } - /// Sets the fast VM modes used by this executor. - pub fn set_fast_vm_modes(&mut self, fast_vm_modes: OneshotExecutorVmModes) { - if fast_vm_modes.uses_fast_vm() { + /// Sets the fast VM mode used by this executor. + pub fn set_fast_vm_mode(&mut self, fast_vm_mode: FastVmMode) { + if !matches!(fast_vm_mode, FastVmMode::Old) { tracing::warn!( - "Running new VM with modes {fast_vm_modes:?}; this can lead to incorrect node behavior" + "Running new VM with modes {fast_vm_mode:?}; this can lead to incorrect node behavior" ); } - self.fast_vm_modes = fast_vm_modes; + self.fast_vm_mode = fast_vm_mode; } /// Causes the VM to panic on divergence whenever it executes in the shadow mode. By default, a divergence is logged on `ERROR` level. @@ -150,7 +112,7 @@ impl MainOneshotExecutor { if tracing_params.trace_calls || !is_supported_by_fast_vm(env.system.version) { FastVmMode::Old // the fast VM doesn't support call tracing or old protocol versions } else { - self.fast_vm_modes.get(env.system.execution_mode) + self.fast_vm_mode } } } diff --git a/core/lib/vm_executor/src/oneshot/tests.rs b/core/lib/vm_executor/src/oneshot/tests.rs index e2ad9e1c72d7..65d2ff3727c0 100644 --- a/core/lib/vm_executor/src/oneshot/tests.rs +++ b/core/lib/vm_executor/src/oneshot/tests.rs @@ -20,7 +20,7 @@ const EXEC_MODES: [TxExecutionMode; 3] = [ #[test] fn selecting_vm_for_execution() { let mut executor = MainOneshotExecutor::new(usize::MAX); - executor.set_fast_vm_modes(FastVmMode::New.into()); + executor.set_fast_vm_mode(FastVmMode::New); for exec_mode in EXEC_MODES { let env = OneshotEnv { @@ -41,25 +41,6 @@ fn selecting_vm_for_execution() { let mode = executor.select_fast_vm_mode(&old_env, &OneshotTracingParams::default()); assert_matches!(mode, FastVmMode::Old); } - - executor.set_fast_vm_modes(OneshotExecutorVmModes { - gas_estimation: FastVmMode::New, - ..OneshotExecutorVmModes::default() - }); - - let mut env = OneshotEnv { - system: default_system_env(TxExecutionMode::EstimateFee), - l1_batch: default_l1_batch_env(1), - current_block: None, - }; - let mode = executor.select_fast_vm_mode(&env, &OneshotTracingParams::default()); - assert_matches!(mode, FastVmMode::New); - - for exec_mode in [TxExecutionMode::VerifyExecute, TxExecutionMode::EthCall] { - env.system.execution_mode = exec_mode; - let mode = executor.select_fast_vm_mode(&env, &OneshotTracingParams::default()); - assert_matches!(mode, FastVmMode::Old); - } } #[test] @@ -115,7 +96,7 @@ async fn inspecting_transfer(exec_mode: TxExecutionMode, fast_vm_mode: FastVmMod let tracing = OneshotTracingParams::default(); let mut executor = MainOneshotExecutor::new(usize::MAX); - executor.set_fast_vm_modes(fast_vm_mode.into()); + executor.set_fast_vm_mode(fast_vm_mode); let result = executor .inspect_transaction_with_bytecode_compression(storage, env, args, tracing) .await diff --git a/core/node/api_server/src/execution_sandbox/execute.rs b/core/node/api_server/src/execution_sandbox/execute.rs index 01f08ab9ab23..7958b5ed3c12 100644 --- a/core/node/api_server/src/execution_sandbox/execute.rs +++ b/core/node/api_server/src/execution_sandbox/execute.rs @@ -108,7 +108,7 @@ impl SandboxExecutor { missed_storage_invocation_limit: usize, ) -> Self { let mut executor = MainOneshotExecutor::new(missed_storage_invocation_limit); - executor.set_fast_vm_modes(options.fast_vm_modes); + executor.set_fast_vm_mode(options.fast_vm_mode); #[cfg(test)] executor.panic_on_divergence(); executor diff --git a/core/node/api_server/src/tx_sender/mod.rs b/core/node/api_server/src/tx_sender/mod.rs index 8c7321681cb1..75cc1ad602f8 100644 --- a/core/node/api_server/src/tx_sender/mod.rs +++ b/core/node/api_server/src/tx_sender/mod.rs @@ -32,7 +32,6 @@ use zksync_types::{ use zksync_utils::h256_to_u256; use zksync_vm_executor::oneshot::{ CallOrExecute, EstimateGas, MultiVMBaseSystemContracts, OneshotEnvParameters, - OneshotExecutorVmModes, }; pub(super) use self::{gas_estimation::BinarySearchKind, result::SubmitTxError}; @@ -91,7 +90,7 @@ pub async fn build_tx_sender( /// Oneshot executor options used by the API server sandbox. #[derive(Debug)] pub struct SandboxExecutorOptions { - pub(crate) fast_vm_modes: OneshotExecutorVmModes, + pub(crate) fast_vm_mode: FastVmMode, /// Env parameters to be used when estimating gas. pub(crate) estimate_gas: OneshotEnvParameters, /// Env parameters to be used when performing `eth_call` requests. @@ -117,7 +116,7 @@ impl SandboxExecutorOptions { .context("failed loading base contracts for calls / tx execution")?; Ok(Self { - fast_vm_modes: FastVmMode::Old.into(), + fast_vm_mode: FastVmMode::Old, estimate_gas: OneshotEnvParameters::new( Arc::new(estimate_gas_contracts), chain_id, @@ -133,9 +132,9 @@ impl SandboxExecutorOptions { }) } - /// Sets the fast VM modes used by this executor. - pub fn set_fast_vm_modes(&mut self, fast_vm_modes: OneshotExecutorVmModes) { - self.fast_vm_modes = fast_vm_modes; + /// Sets the fast VM mode used by this executor. + pub fn set_fast_vm_mode(&mut self, fast_vm_mode: FastVmMode) { + self.fast_vm_mode = fast_vm_mode; } pub(crate) async fn mock() -> Self { diff --git a/core/node/api_server/src/tx_sender/tests/mod.rs b/core/node/api_server/src/tx_sender/tests/mod.rs index 6ab27c8029e3..ea3f77fbcd82 100644 --- a/core/node/api_server/src/tx_sender/tests/mod.rs +++ b/core/node/api_server/src/tx_sender/tests/mod.rs @@ -152,7 +152,7 @@ async fn create_real_tx_sender(pool: ConnectionPool) -> TxSender { ) .await .unwrap(); - executor_options.set_fast_vm_modes(FastVmMode::Shadow.into()); + executor_options.set_fast_vm_mode(FastVmMode::Shadow); let pg_caches = PostgresStorageCaches::new(1, 1); let tx_executor = SandboxExecutor::real(executor_options, pg_caches, usize::MAX); diff --git a/core/node/api_server/src/web3/tests/vm.rs b/core/node/api_server/src/web3/tests/vm.rs index 0e59bd481ea9..7dd0164198a1 100644 --- a/core/node/api_server/src/web3/tests/vm.rs +++ b/core/node/api_server/src/web3/tests/vm.rs @@ -92,7 +92,7 @@ impl BaseSystemContractsProvider for BaseContractsWithMockE fn executor_options_with_evm_emulator() -> SandboxExecutorOptions { let base_contracts = Arc::::default(); SandboxExecutorOptions { - fast_vm_modes: FastVmMode::Old.into(), + fast_vm_mode: FastVmMode::Old, estimate_gas: OneshotEnvParameters::new( base_contracts.clone(), L2ChainId::default(), diff --git a/core/node/node_framework/src/implementations/layers/web3_api/tx_sender.rs b/core/node/node_framework/src/implementations/layers/web3_api/tx_sender.rs index 727d7c0fbeaa..023ef1059c79 100644 --- a/core/node/node_framework/src/implementations/layers/web3_api/tx_sender.rs +++ b/core/node/node_framework/src/implementations/layers/web3_api/tx_sender.rs @@ -7,7 +7,6 @@ use zksync_node_api_server::{ }; use zksync_state::{PostgresStorageCaches, PostgresStorageCachesTask}; use zksync_types::{vm::FastVmMode, AccountTreeId, Address}; -use zksync_vm_executor::oneshot::OneshotExecutorVmModes; use zksync_web3_decl::{ client::{DynClient, L2}, jsonrpsee, @@ -61,7 +60,7 @@ pub struct TxSenderLayer { postgres_storage_caches_config: PostgresStorageCachesConfig, max_vm_concurrency: usize, whitelisted_tokens_for_aa_cache: bool, - vm_modes: OneshotExecutorVmModes, + vm_mode: FastVmMode, } #[derive(Debug, FromContext)] @@ -97,7 +96,7 @@ impl TxSenderLayer { postgres_storage_caches_config, max_vm_concurrency, whitelisted_tokens_for_aa_cache: false, - vm_modes: OneshotExecutorVmModes::default(), + vm_mode: FastVmMode::Old, } } @@ -110,9 +109,9 @@ impl TxSenderLayer { self } - /// Sets the fast VM modes used for gas estimation. - pub fn with_gas_estimation_vm_mode(mut self, mode: FastVmMode) -> Self { - self.vm_modes.gas_estimation = mode; + /// Sets the fast VM modes used for all supported operations. + pub fn with_vm_mode(mut self, mode: FastVmMode) -> Self { + self.vm_mode = mode; self } } @@ -166,7 +165,7 @@ impl WiringLayer for TxSenderLayer { config.validation_computational_gas_limit, ) .await?; - executor_options.set_fast_vm_modes(self.vm_modes); + executor_options.set_fast_vm_mode(self.vm_mode); // Build `TxSender`. let mut tx_sender = TxSenderBuilder::new(config, replica_pool, tx_sink); From 79cd20593485eb72e863e03ed5acdac7eab4c5c3 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 21 Oct 2024 13:30:27 +0300 Subject: [PATCH 17/18] Update config parameter naming --- core/lib/env_config/src/vm_runner.rs | 2 +- etc/env/file_based/overrides/tests/integration.yaml | 2 +- etc/env/file_based/overrides/tests/loadtest-new.yaml | 2 +- etc/env/file_based/overrides/tests/loadtest-old.yaml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/lib/env_config/src/vm_runner.rs b/core/lib/env_config/src/vm_runner.rs index 944dd0f98e9b..0a29d1256bd2 100644 --- a/core/lib/env_config/src/vm_runner.rs +++ b/core/lib/env_config/src/vm_runner.rs @@ -55,7 +55,7 @@ mod tests { let mut lock = MUTEX.lock(); let config = r#" EXPERIMENTAL_VM_STATE_KEEPER_FAST_VM_MODE=new - EXPERIMENTAL_VM_API_FAST_VM_MODE_FOR_GAS_ESTIMATION=shadow + EXPERIMENTAL_VM_API_FAST_VM_MODE=shadow EXPERIMENTAL_VM_PLAYGROUND_FAST_VM_MODE=shadow EXPERIMENTAL_VM_PLAYGROUND_DB_PATH=/db/vm_playground EXPERIMENTAL_VM_PLAYGROUND_FIRST_PROCESSED_BATCH=123 diff --git a/etc/env/file_based/overrides/tests/integration.yaml b/etc/env/file_based/overrides/tests/integration.yaml index 52bd4a3a5c95..6ad031e29458 100644 --- a/etc/env/file_based/overrides/tests/integration.yaml +++ b/etc/env/file_based/overrides/tests/integration.yaml @@ -1,4 +1,4 @@ experimental_vm: # Use the shadow VM mode everywhere to catch divergences as early as possible state_keeper_fast_vm_mode: SHADOW - api_fast_vm_mode_for_gas_estimation: SHADOW + api_fast_vm_mode: SHADOW diff --git a/etc/env/file_based/overrides/tests/loadtest-new.yaml b/etc/env/file_based/overrides/tests/loadtest-new.yaml index 3d51cbb322da..e66625636b1f 100644 --- a/etc/env/file_based/overrides/tests/loadtest-new.yaml +++ b/etc/env/file_based/overrides/tests/loadtest-new.yaml @@ -6,6 +6,6 @@ api: estimate_gas_optimize_search: true experimental_vm: state_keeper_fast_vm_mode: NEW - api_fast_vm_mode_for_gas_estimation: NEW + api_fast_vm_mode: NEW mempool: delay_interval: 50 diff --git a/etc/env/file_based/overrides/tests/loadtest-old.yaml b/etc/env/file_based/overrides/tests/loadtest-old.yaml index f4336199842e..7b1a35870187 100644 --- a/etc/env/file_based/overrides/tests/loadtest-old.yaml +++ b/etc/env/file_based/overrides/tests/loadtest-old.yaml @@ -3,6 +3,6 @@ db: mode: LIGHTWEIGHT experimental_vm: state_keeper_fast_vm_mode: OLD - api_fast_vm_mode_for_gas_estimation: OLD + api_fast_vm_mode: OLD mempool: delay_interval: 50 From 69d76a5996c974c1e684a5451c45aed1c5ee8865 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 24 Oct 2024 13:51:35 +0300 Subject: [PATCH 18/18] Increase expected TPS for load test with new VM --- .github/workflows/ci-core-reusable.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-core-reusable.yml b/.github/workflows/ci-core-reusable.yml index 8ca1c0b69b28..6bb48b0974e5 100644 --- a/.github/workflows/ci-core-reusable.yml +++ b/.github/workflows/ci-core-reusable.yml @@ -101,7 +101,7 @@ jobs: - name: Loadtest configuration run: | - echo EXPECTED_TX_COUNT=${{ matrix.vm_mode == 'NEW' && 21000 || 16000 }} >> .env + echo EXPECTED_TX_COUNT=${{ matrix.vm_mode == 'NEW' && 30000 || 16000 }} >> .env echo ACCOUNTS_AMOUNT="100" >> .env echo MAX_INFLIGHT_TXS="10" >> .env echo SYNC_API_REQUESTS_LIMIT="15" >> .env