From 6bbe2d65cf606599dca37dc9c16318de3db8f34b Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 2 Sep 2024 16:31:27 +0300 Subject: [PATCH 01/11] Sketch moving oneshot executor --- Cargo.lock | 2 + core/lib/vm_executor/Cargo.toml | 1 + core/lib/vm_executor/src/batch/factory.rs | 4 +- core/lib/vm_executor/src/batch/metrics.rs | 9 +- core/lib/vm_executor/src/lib.rs | 2 + core/lib/vm_executor/src/oneshot/metrics.rs | 143 +++++++++++ .../vm_executor/src/oneshot/mock.rs} | 29 +-- core/lib/vm_executor/src/oneshot/mod.rs | 243 ++++++++++++++++++ .../vm_executor/src/oneshot}/tracers.rs | 2 +- core/lib/vm_executor/src/shared.rs | 12 + core/lib/vm_interface/src/executor.rs | 30 ++- core/lib/vm_interface/src/lib.rs | 4 +- core/lib/vm_interface/src/types/inputs/mod.rs | 65 +++++ core/node/api_server/Cargo.toml | 1 + .../api_server/src/execution_sandbox/apply.rs | 238 +---------------- .../src/execution_sandbox/execute.rs | 84 +----- .../api_server/src/execution_sandbox/mod.rs | 56 +--- .../api_server/src/execution_sandbox/tests.rs | 9 +- .../src/execution_sandbox/validate.rs | 5 +- .../src/execution_sandbox/vm_metrics.rs | 144 +---------- core/node/api_server/src/tx_sender/mod.rs | 8 +- core/node/api_server/src/tx_sender/tests.rs | 6 +- .../api_server/src/web3/namespaces/debug.rs | 5 +- core/node/api_server/src/web3/testonly.rs | 6 +- core/node/api_server/src/web3/tests/mod.rs | 6 +- core/node/api_server/src/web3/tests/vm.rs | 1 + 26 files changed, 553 insertions(+), 562 deletions(-) create mode 100644 core/lib/vm_executor/src/oneshot/metrics.rs rename core/{node/api_server/src/execution_sandbox/testonly.rs => lib/vm_executor/src/oneshot/mock.rs} (78%) create mode 100644 core/lib/vm_executor/src/oneshot/mod.rs rename core/{node/api_server/src/execution_sandbox => lib/vm_executor/src/oneshot}/tracers.rs (98%) create mode 100644 core/lib/vm_executor/src/shared.rs diff --git a/Cargo.lock b/Cargo.lock index cfa18534528..a9234e6f447 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9043,6 +9043,7 @@ dependencies = [ "zksync_system_constants", "zksync_types", "zksync_utils", + "zksync_vm_executor", "zksync_web3_decl", ] @@ -9803,6 +9804,7 @@ dependencies = [ "zksync_dal", "zksync_multivm", "zksync_types", + "zksync_utils", ] [[package]] diff --git a/core/lib/vm_executor/Cargo.toml b/core/lib/vm_executor/Cargo.toml index 9471e263bf4..089c2a9bcca 100644 --- a/core/lib/vm_executor/Cargo.toml +++ b/core/lib/vm_executor/Cargo.toml @@ -15,6 +15,7 @@ zksync_contracts.workspace = true zksync_dal.workspace = true zksync_types.workspace = true zksync_multivm.workspace = true +zksync_utils.workspace = true async-trait.workspace = true once_cell.workspace = true diff --git a/core/lib/vm_executor/src/batch/factory.rs b/core/lib/vm_executor/src/batch/factory.rs index 17b125b0c41..789d525d669 100644 --- a/core/lib/vm_executor/src/batch/factory.rs +++ b/core/lib/vm_executor/src/batch/factory.rs @@ -18,9 +18,9 @@ use zksync_types::{vm::FastVmMode, Transaction}; use super::{ executor::{Command, MainBatchExecutor}, - metrics::{TxExecutionStage, BATCH_TIP_METRICS, KEEPER_METRICS}, + metrics::{TxExecutionStage, BATCH_TIP_METRICS, EXECUTOR_METRICS, KEEPER_METRICS}, }; -use crate::batch::metrics::{InteractionType, EXECUTOR_METRICS}; +use crate::shared::InteractionType; /// The default implementation of [`BatchExecutorFactory`]. /// Creates real batch executors which maintain the VM (as opposed to the test factories which don't use the VM). diff --git a/core/lib/vm_executor/src/batch/metrics.rs b/core/lib/vm_executor/src/batch/metrics.rs index 170ed471798..6851193e9be 100644 --- a/core/lib/vm_executor/src/batch/metrics.rs +++ b/core/lib/vm_executor/src/batch/metrics.rs @@ -5,6 +5,8 @@ use std::time::Duration; use vise::{Buckets, EncodeLabelSet, EncodeLabelValue, Family, Histogram, Metrics}; use zksync_multivm::interface::VmExecutionResultAndLogs; +use crate::shared::InteractionType; + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, EncodeLabelValue, EncodeLabelSet)] #[metrics(label = "command", rename_all = "snake_case")] pub(super) enum ExecutorCommand { @@ -26,13 +28,6 @@ pub(super) enum TxExecutionStage { TxRollback, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, EncodeLabelValue, EncodeLabelSet)] -#[metrics(label = "interaction", rename_all = "snake_case")] -pub(super) enum InteractionType { - GetValue, - SetValue, -} - /// Executor-related metrics. #[derive(Debug, Metrics)] #[metrics(prefix = "state_keeper")] diff --git a/core/lib/vm_executor/src/lib.rs b/core/lib/vm_executor/src/lib.rs index 24fb3d8f7ee..1a0fbb002df 100644 --- a/core/lib/vm_executor/src/lib.rs +++ b/core/lib/vm_executor/src/lib.rs @@ -6,4 +6,6 @@ pub use zksync_multivm::interface::executor as interface; pub mod batch; +pub mod oneshot; +mod shared; pub mod storage; diff --git a/core/lib/vm_executor/src/oneshot/metrics.rs b/core/lib/vm_executor/src/oneshot/metrics.rs new file mode 100644 index 00000000000..8a89ce0a9a4 --- /dev/null +++ b/core/lib/vm_executor/src/oneshot/metrics.rs @@ -0,0 +1,143 @@ +use std::time::Duration; + +use vise::{Buckets, EncodeLabelSet, EncodeLabelValue, Family, Histogram, Metrics}; +use zksync_multivm::interface::{storage::StorageViewMetrics, VmMemoryMetrics}; + +use crate::shared::InteractionType; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, EncodeLabelValue, EncodeLabelSet)] +#[metrics(label = "type", rename_all = "snake_case")] +enum SizeType { + Inner, + History, +} + +const MEMORY_SIZE_BUCKETS: Buckets = Buckets::values(&[ + 1_000.0, + 10_000.0, + 100_000.0, + 500_000.0, + 1_000_000.0, + 5_000_000.0, + 10_000_000.0, + 50_000_000.0, + 100_000_000.0, + 500_000_000.0, + 1_000_000_000.0, +]); + +#[derive(Debug, Metrics)] +#[metrics(prefix = "runtime_context_memory")] +struct RuntimeContextMemoryMetrics { + #[metrics(buckets = MEMORY_SIZE_BUCKETS)] + event_sink_size: Family>, + #[metrics(buckets = MEMORY_SIZE_BUCKETS)] + memory_size: Family>, + #[metrics(buckets = MEMORY_SIZE_BUCKETS)] + decommitter_size: Family>, + #[metrics(buckets = MEMORY_SIZE_BUCKETS)] + storage_size: Family>, + #[metrics(buckets = MEMORY_SIZE_BUCKETS)] + storage_view_cache_size: Histogram, + #[metrics(buckets = MEMORY_SIZE_BUCKETS)] + full: Histogram, +} + +#[vise::register] +static MEMORY_METRICS: vise::Global = vise::Global::new(); + +const INTERACTION_AMOUNT_BUCKETS: Buckets = Buckets::exponential(10.0..=10_000_000.0, 10.0); + +#[derive(Debug, Metrics)] +#[metrics(prefix = "runtime_context_storage_interaction")] +struct RuntimeContextStorageMetrics { + #[metrics(buckets = INTERACTION_AMOUNT_BUCKETS)] + amount: Family>, + #[metrics(buckets = Buckets::LATENCIES)] + duration: Family>, + #[metrics(buckets = Buckets::LATENCIES)] + duration_per_unit: Family>, + #[metrics(buckets = Buckets::ZERO_TO_ONE)] + ratio: Histogram, +} + +#[vise::register] +static STORAGE_METRICS: vise::Global = vise::Global::new(); + +pub(super) fn report_vm_memory_metrics( + tx_id: &str, + memory_metrics: &VmMemoryMetrics, + vm_execution_took: Duration, + storage_metrics: StorageViewMetrics, +) { + 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); + MEMORY_METRICS.memory_size[&SizeType::Inner].observe(memory_metrics.memory_inner); + MEMORY_METRICS.memory_size[&SizeType::History].observe(memory_metrics.memory_history); + MEMORY_METRICS.decommitter_size[&SizeType::Inner] + .observe(memory_metrics.decommittment_processor_inner); + MEMORY_METRICS.decommitter_size[&SizeType::History] + .observe(memory_metrics.decommittment_processor_history); + MEMORY_METRICS.storage_size[&SizeType::Inner].observe(memory_metrics.storage_inner); + MEMORY_METRICS.storage_size[&SizeType::History].observe(memory_metrics.storage_history); + + MEMORY_METRICS + .storage_view_cache_size + .observe(storage_metrics.cache_size); + MEMORY_METRICS + .full + .observe(memory_metrics.full_size() + storage_metrics.cache_size); + + let total_storage_invocations = storage_metrics.get_value_storage_invocations + + storage_metrics.set_value_storage_invocations; + let total_time_spent_in_storage = + storage_metrics.time_spent_on_get_value + storage_metrics.time_spent_on_set_value; + + STORAGE_METRICS.amount[&InteractionType::Missed] + .observe(storage_metrics.storage_invocations_missed); + STORAGE_METRICS.amount[&InteractionType::GetValue] + .observe(storage_metrics.get_value_storage_invocations); + STORAGE_METRICS.amount[&InteractionType::SetValue] + .observe(storage_metrics.set_value_storage_invocations); + STORAGE_METRICS.amount[&InteractionType::Total].observe(total_storage_invocations); + + STORAGE_METRICS.duration[&InteractionType::Missed] + .observe(storage_metrics.time_spent_on_storage_missed); + STORAGE_METRICS.duration[&InteractionType::GetValue] + .observe(storage_metrics.time_spent_on_get_value); + STORAGE_METRICS.duration[&InteractionType::SetValue] + .observe(storage_metrics.time_spent_on_set_value); + STORAGE_METRICS.duration[&InteractionType::Total].observe(total_time_spent_in_storage); + + if total_storage_invocations > 0 { + STORAGE_METRICS.duration_per_unit[&InteractionType::Total] + .observe(total_time_spent_in_storage.div_f64(total_storage_invocations as f64)); + } + if storage_metrics.storage_invocations_missed > 0 { + let duration_per_unit = storage_metrics + .time_spent_on_storage_missed + .div_f64(storage_metrics.storage_invocations_missed as f64); + STORAGE_METRICS.duration_per_unit[&InteractionType::Missed].observe(duration_per_unit); + } + + STORAGE_METRICS + .ratio + .observe(total_time_spent_in_storage.as_secs_f64() / vm_execution_took.as_secs_f64()); + + const STORAGE_INVOCATIONS_DEBUG_THRESHOLD: usize = 1_000; + + if total_storage_invocations > STORAGE_INVOCATIONS_DEBUG_THRESHOLD { + tracing::info!( + "Tx {tx_id} resulted in {total_storage_invocations} storage_invocations, {} new_storage_invocations, \ + {} get_value_storage_invocations, {} set_value_storage_invocations, \ + vm execution took {vm_execution_took:?}, storage interaction took {total_time_spent_in_storage:?} \ + (missed: {:?} get: {:?} set: {:?})", + storage_metrics.storage_invocations_missed, + storage_metrics.get_value_storage_invocations, + storage_metrics.set_value_storage_invocations, + storage_metrics.time_spent_on_storage_missed, + storage_metrics.time_spent_on_get_value, + storage_metrics.time_spent_on_set_value, + ); + } +} diff --git a/core/node/api_server/src/execution_sandbox/testonly.rs b/core/lib/vm_executor/src/oneshot/mock.rs similarity index 78% rename from core/node/api_server/src/execution_sandbox/testonly.rs rename to core/lib/vm_executor/src/oneshot/mock.rs index d9d60f52415..5e84605f99c 100644 --- a/core/node/api_server/src/execution_sandbox/testonly.rs +++ b/core/lib/vm_executor/src/oneshot/mock.rs @@ -1,16 +1,12 @@ use std::fmt; use async_trait::async_trait; -#[cfg(test)] -use zksync_multivm::interface::ExecutionResult; use zksync_multivm::interface::{ - storage::ReadStorage, BytecodeCompressionError, OneshotEnv, TxExecutionMode, - VmExecutionResultAndLogs, + executor::OneshotExecutor, storage::ReadStorage, BytecodeCompressionError, ExecutionResult, + OneshotEnv, TxExecutionArgs, TxExecutionMode, VmExecutionResultAndLogs, }; use zksync_types::Transaction; -use super::{execute::TransactionExecutor, OneshotExecutor, TxExecutionArgs}; - type TxResponseFn = dyn Fn(&Transaction, &OneshotEnv) -> VmExecutionResultAndLogs + Send + Sync; pub struct MockOneshotExecutor { @@ -30,10 +26,7 @@ impl Default for MockOneshotExecutor { fn default() -> Self { Self { call_responses: Box::new(|tx, _| { - panic!( - "Unexpected call with data {}", - hex::encode(tx.execute.calldata()) - ); + panic!("Unexpected call with data {:?}", tx.execute.calldata()); }), tx_responses: Box::new(|tx, _| { panic!("Unexpect transaction call: {tx:?}"); @@ -43,23 +36,20 @@ impl Default for MockOneshotExecutor { } impl MockOneshotExecutor { - #[cfg(test)] - pub(crate) fn set_call_responses(&mut self, responses: F) + pub fn set_call_responses(&mut self, responses: F) where F: Fn(&Transaction, &OneshotEnv) -> ExecutionResult + 'static + Send + Sync, { self.call_responses = self.wrap_responses(responses); } - #[cfg(test)] - pub(crate) fn set_tx_responses(&mut self, responses: F) + pub fn set_tx_responses(&mut self, responses: F) where F: Fn(&Transaction, &OneshotEnv) -> ExecutionResult + 'static + Send + Sync, { self.tx_responses = self.wrap_responses(responses); } - #[cfg(test)] fn wrap_responses(&mut self, responses: F) -> Box where F: Fn(&Transaction, &OneshotEnv) -> ExecutionResult + 'static + Send + Sync, @@ -76,8 +66,7 @@ impl MockOneshotExecutor { ) } - #[cfg(test)] - pub(crate) fn set_tx_responses_with_logs(&mut self, responses: F) + pub fn set_tx_responses_with_logs(&mut self, responses: F) where F: Fn(&Transaction, &OneshotEnv) -> VmExecutionResultAndLogs + 'static + Send + Sync, { @@ -124,9 +113,3 @@ where Ok((Ok(()), self.mock_inspect(env, args))) } } - -impl From for TransactionExecutor { - fn from(executor: MockOneshotExecutor) -> Self { - Self::Mock(executor) - } -} diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs new file mode 100644 index 00000000000..65a574a4e60 --- /dev/null +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -0,0 +1,243 @@ +//! Oneshot VM executor. + +use std::time::{Duration, Instant}; + +use anyhow::Context; +use async_trait::async_trait; +use zksync_multivm::{ + interface::{ + executor::OneshotExecutor, + storage::{ReadStorage, StoragePtr, StorageView, WriteStorage}, + BytecodeCompressionError, OneshotEnv, StoredL2BlockEnv, TxExecutionArgs, TxExecutionMode, + VmExecutionMode, VmExecutionResultAndLogs, VmInterface, + }, + tracers::StorageInvocations, + utils::adjust_pubdata_price_for_tx, + vm_latest::HistoryDisabled, + zk_evm_latest::ethereum_types::U256, + MultiVMTracer, MultiVmTracerPointer, VmInstance, +}; +use zksync_types::{ + block::pack_block_info, + get_nonce_key, + utils::{decompose_full_nonce, nonces_to_full_nonce, storage_key_for_eth_balance}, + AccountTreeId, Nonce, StorageKey, Transaction, SYSTEM_CONTEXT_ADDRESS, + SYSTEM_CONTEXT_CURRENT_L2_BLOCK_INFO_POSITION, SYSTEM_CONTEXT_CURRENT_TX_ROLLING_HASH_POSITION, +}; +use zksync_utils::{h256_to_u256, u256_to_h256}; + +pub use self::{mock::MockOneshotExecutor, tracers::ApiTracer}; + +mod metrics; +mod mock; +mod tracers; + +/// Main [`OneshotExecutor`] implementation used by the API server. +#[derive(Debug, Default)] +pub struct MainOneshotExecutor { + missed_storage_invocation_limit: usize, +} + +impl MainOneshotExecutor { + /// Creates a new executor with the specified limit of cache misses for storage read operations (an anti-DoS measure). + /// The limit is applied for calls and gas estimations, but not during transaction validation. + pub fn new(missed_storage_invocation_limit: usize) -> Self { + Self { + missed_storage_invocation_limit, + } + } +} + +#[async_trait] +impl OneshotExecutor for MainOneshotExecutor +where + S: ReadStorage + Send + 'static, +{ + type Tracers = Vec; + + async fn inspect_transaction( + &self, + storage: S, + env: OneshotEnv, + args: TxExecutionArgs, + tracers: Self::Tracers, + ) -> anyhow::Result { + let missed_storage_invocation_limit = match env.system.execution_mode { + // storage accesses are not limited for tx validation + TxExecutionMode::VerifyExecute => usize::MAX, + TxExecutionMode::EthCall | TxExecutionMode::EstimateFee => { + self.missed_storage_invocation_limit + } + }; + + tokio::task::spawn_blocking(move || { + let tracers = VmSandbox::wrap_tracers(tracers, &env, missed_storage_invocation_limit); + let executor = VmSandbox::new(storage, env, args); + executor.apply(|vm, transaction| { + vm.push_transaction(transaction); + vm.inspect(tracers.into(), VmExecutionMode::OneTx) + }) + }) + .await + .context("VM execution panicked") + } + + async fn inspect_transaction_with_bytecode_compression( + &self, + storage: S, + env: OneshotEnv, + args: TxExecutionArgs, + tracers: Self::Tracers, + ) -> anyhow::Result<( + Result<(), BytecodeCompressionError>, + VmExecutionResultAndLogs, + )> { + let missed_storage_invocation_limit = match env.system.execution_mode { + // storage accesses are not limited for tx validation + TxExecutionMode::VerifyExecute => usize::MAX, + TxExecutionMode::EthCall | TxExecutionMode::EstimateFee => { + self.missed_storage_invocation_limit + } + }; + + tokio::task::spawn_blocking(move || { + let tracers = VmSandbox::wrap_tracers(tracers, &env, missed_storage_invocation_limit); + let executor = VmSandbox::new(storage, env, args); + executor.apply(|vm, transaction| { + let (bytecodes_result, exec_result) = vm + .inspect_transaction_with_bytecode_compression( + tracers.into(), + transaction, + true, + ); + (bytecodes_result.map(drop), exec_result) + }) + }) + .await + .context("VM execution panicked") + } +} + +#[derive(Debug)] +struct VmSandbox { + vm: Box>, + storage_view: StoragePtr>, + transaction: Transaction, +} + +impl VmSandbox { + /// This method is blocking. + pub fn new(storage: S, mut env: OneshotEnv, execution_args: TxExecutionArgs) -> Self { + let mut storage_view = StorageView::new(storage); + Self::setup_storage_view(&mut storage_view, &execution_args, env.current_block); + + 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 = storage_view.to_rc_ptr(); + let vm = Box::new(VmInstance::new_with_specific_version( + env.l1_batch, + env.system, + storage_view.clone(), + protocol_version.into_api_vm_version(), + )); + + Self { + vm, + storage_view, + transaction: execution_args.transaction, + } + } + + /// This method is blocking. + fn setup_storage_view( + storage_view: &mut StorageView, + 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 (_, 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)); + } + + 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)); + current_balance += execution_args.added_balance; + storage_view.set_value(balance_key, u256_to_h256(current_balance)); + + // Reset L2 block info if necessary. + if let Some(current_block) = current_block { + let l2_block_info_key = StorageKey::new( + AccountTreeId::new(SYSTEM_CONTEXT_ADDRESS), + SYSTEM_CONTEXT_CURRENT_L2_BLOCK_INFO_POSITION, + ); + 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)); + + 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( + l2_block_txs_rolling_hash_key, + current_block.txs_rolling_hash, + ); + } + + let storage_view_setup_time = storage_view_setup_started_at.elapsed(); + // We don't want to emit too many logs. + if storage_view_setup_time > Duration::from_millis(10) { + tracing::debug!("Prepared the storage view (took {storage_view_setup_time:?})",); + } + } + + fn wrap_tracers( + tracers: Vec, + env: &OneshotEnv, + missed_storage_invocation_limit: usize, + ) -> Vec, HistoryDisabled>> { + let storage_invocation_tracer = StorageInvocations::new(missed_storage_invocation_limit); + let protocol_version = env.system.version; + tracers + .into_iter() + .map(|tracer| tracer.into_boxed(protocol_version)) + .chain([storage_invocation_tracer.into_tracer_pointer()]) + .collect() + } + + pub(super) fn apply(mut self, apply_fn: F) -> T + where + F: FnOnce(&mut VmInstance, Transaction) -> T, + { + let tx_id = format!( + "{:?}-{}", + self.transaction.initiator_account(), + self.transaction.nonce().unwrap_or(Nonce(0)) + ); + + let result = apply_fn(&mut *self.vm, self.transaction); + let vm_execution_took = Duration::ZERO; // FIXME: metric + + let memory_metrics = self.vm.record_vm_memory_metrics(); + metrics::report_vm_memory_metrics( + &tx_id, + &memory_metrics, + vm_execution_took, + self.storage_view.as_ref().borrow_mut().metrics(), + ); + result + } +} diff --git a/core/node/api_server/src/execution_sandbox/tracers.rs b/core/lib/vm_executor/src/oneshot/tracers.rs similarity index 98% rename from core/node/api_server/src/execution_sandbox/tracers.rs rename to core/lib/vm_executor/src/oneshot/tracers.rs index 31384b7a089..6fdc3dbc7b6 100644 --- a/core/node/api_server/src/execution_sandbox/tracers.rs +++ b/core/lib/vm_executor/src/oneshot/tracers.rs @@ -11,7 +11,7 @@ use zksync_types::ProtocolVersionId; /// Custom tracers supported by the API sandbox. #[derive(Debug)] -pub(crate) enum ApiTracer { +pub enum ApiTracer { CallTracer(Arc>>), Validation { params: ValidationTracerParams, diff --git a/core/lib/vm_executor/src/shared.rs b/core/lib/vm_executor/src/shared.rs new file mode 100644 index 00000000000..420005be05d --- /dev/null +++ b/core/lib/vm_executor/src/shared.rs @@ -0,0 +1,12 @@ +//! Functionality shared among different types of executors. + +use vise::{EncodeLabelSet, EncodeLabelValue}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, EncodeLabelValue, EncodeLabelSet)] +#[metrics(label = "interaction", rename_all = "snake_case")] +pub(crate) enum InteractionType { + Missed, + GetValue, + SetValue, + Total, +} diff --git a/core/lib/vm_interface/src/executor.rs b/core/lib/vm_interface/src/executor.rs index ee6665abfcb..f6aad29387c 100644 --- a/core/lib/vm_interface/src/executor.rs +++ b/core/lib/vm_interface/src/executor.rs @@ -6,8 +6,9 @@ use async_trait::async_trait; use zksync_types::Transaction; use crate::{ - storage::StorageView, BatchTransactionExecutionResult, FinishedL1Batch, L1BatchEnv, L2BlockEnv, - SystemEnv, + storage::{ReadStorage, StorageView}, + BatchTransactionExecutionResult, BytecodeCompressionError, FinishedL1Batch, L1BatchEnv, + L2BlockEnv, OneshotEnv, SystemEnv, TxExecutionArgs, VmExecutionResultAndLogs, }; /// Factory of [`BatchExecutor`]s. @@ -42,3 +43,28 @@ pub trait BatchExecutor: 'static + Send + fmt::Debug { /// Finished the current L1 batch. async fn finish_batch(self: Box) -> anyhow::Result<(FinishedL1Batch, StorageView)>; } + +/// VM executor capable of executing isolated transactions / calls (as opposed to [batch execution](BatchExecutor)). +#[async_trait] +pub trait OneshotExecutor { + type Tracers: Default; // FIXME: revise + + async fn inspect_transaction( + &self, + storage: S, + env: OneshotEnv, + args: TxExecutionArgs, + tracers: Self::Tracers, + ) -> anyhow::Result; + + async fn inspect_transaction_with_bytecode_compression( + &self, + storage: S, + env: OneshotEnv, + args: TxExecutionArgs, + tracers: Self::Tracers, + ) -> anyhow::Result<( + Result<(), BytecodeCompressionError>, + VmExecutionResultAndLogs, + )>; +} diff --git a/core/lib/vm_interface/src/lib.rs b/core/lib/vm_interface/src/lib.rs index 315eb2bb36a..333147b4fd7 100644 --- a/core/lib/vm_interface/src/lib.rs +++ b/core/lib/vm_interface/src/lib.rs @@ -24,8 +24,8 @@ pub use crate::{ VmRevertReason, VmRevertReasonParsingError, }, inputs::{ - L1BatchEnv, L2BlockEnv, OneshotEnv, StoredL2BlockEnv, SystemEnv, TxExecutionMode, - VmExecutionMode, + L1BatchEnv, L2BlockEnv, OneshotEnv, StoredL2BlockEnv, SystemEnv, TxExecutionArgs, + TxExecutionMode, VmExecutionMode, }, outputs::{ BatchTransactionExecutionResult, BootloaderMemory, Call, CallType, CircuitStatistic, diff --git a/core/lib/vm_interface/src/types/inputs/mod.rs b/core/lib/vm_interface/src/types/inputs/mod.rs index 4801c4d88b5..871421ac344 100644 --- a/core/lib/vm_interface/src/types/inputs/mod.rs +++ b/core/lib/vm_interface/src/types/inputs/mod.rs @@ -1,3 +1,7 @@ +use zksync_types::{ + l2::L2Tx, ExecuteTransactionCommon, Nonce, PackedEthSignature, Transaction, U256, +}; + pub use self::{ execution_mode::VmExecutionMode, l1_batch_env::L1BatchEnv, @@ -21,3 +25,64 @@ pub struct OneshotEnv { /// in the system context contract, which are set from `L1BatchEnv.first_l2_block` by default. pub current_block: Option, } + +/// Executor-independent arguments necessary to for oneshot transaction execution. +/// +/// # Developer guidelines +/// +/// Please don't add fields that duplicate `SystemEnv` or `L1BatchEnv` information, since both of these +/// are also provided to an executor. +#[derive(Debug)] +pub struct TxExecutionArgs { + /// Transaction / call itself. + pub transaction: Transaction, + /// Nonce override for the initiator account. + pub enforced_nonce: Option, + /// Balance added to the initiator account. + pub added_balance: U256, + /// If `true`, then the batch's L1 / pubdata gas price will be adjusted so that the transaction's gas per pubdata limit is <= + /// to the one in the block. This is often helpful in case we want the transaction validation to work regardless of the + /// current L1 prices for gas or pubdata. + pub adjust_pubdata_price: bool, +} + +impl TxExecutionArgs { + pub fn for_validation(tx: L2Tx) -> Self { + Self { + enforced_nonce: Some(tx.nonce()), + added_balance: U256::zero(), + adjust_pubdata_price: true, + transaction: tx.into(), + } + } + + pub fn for_eth_call(mut call: L2Tx) -> Self { + if call.common_data.signature.is_empty() { + call.common_data.signature = PackedEthSignature::default().serialize_packed().into(); + } + + Self { + enforced_nonce: None, + added_balance: U256::zero(), + adjust_pubdata_price: false, + transaction: call.into(), + } + } + + pub fn for_gas_estimate(transaction: Transaction) -> Self { + // For L2 transactions we need to explicitly put enough balance into the account of the users + // while for L1->L2 transactions the `to_mint` field plays this role + let added_balance = match &transaction.common_data { + ExecuteTransactionCommon::L2(data) => data.fee.gas_limit * data.fee.max_fee_per_gas, + ExecuteTransactionCommon::L1(_) => U256::zero(), + ExecuteTransactionCommon::ProtocolUpgrade(_) => U256::zero(), + }; + + Self { + enforced_nonce: transaction.nonce(), + added_balance, + adjust_pubdata_price: true, + transaction, + } + } +} diff --git a/core/node/api_server/Cargo.toml b/core/node/api_server/Cargo.toml index f7d40210b48..040e2a94a11 100644 --- a/core/node/api_server/Cargo.toml +++ b/core/node/api_server/Cargo.toml @@ -29,6 +29,7 @@ zksync_utils.workspace = true zksync_protobuf.workspace = true zksync_mini_merkle_tree.workspace = true zksync_multivm.workspace = true +zksync_vm_executor.workspace = true vise.workspace = true anyhow.workspace = true diff --git a/core/node/api_server/src/execution_sandbox/apply.rs b/core/node/api_server/src/execution_sandbox/apply.rs index 8b5cf69822b..875b9500af1 100644 --- a/core/node/api_server/src/execution_sandbox/apply.rs +++ b/core/node/api_server/src/execution_sandbox/apply.rs @@ -9,19 +9,12 @@ use std::time::{Duration, Instant}; use anyhow::Context as _; -use async_trait::async_trait; use tokio::runtime::Handle; use zksync_dal::{Connection, Core, CoreDal, DalError}; use zksync_multivm::{ - interface::{ - storage::{ReadStorage, StoragePtr, StorageView, WriteStorage}, - BytecodeCompressionError, L1BatchEnv, L2BlockEnv, OneshotEnv, StoredL2BlockEnv, SystemEnv, - TxExecutionMode, VmExecutionMode, VmExecutionResultAndLogs, VmInterface, - }, - tracers::StorageInvocations, - utils::{adjust_pubdata_price_for_tx, get_eth_call_gas_limit}, - vm_latest::{constants::BATCH_COMPUTATIONAL_GAS_LIMIT, HistoryDisabled}, - MultiVMTracer, MultiVmTracerPointer, VmInstance, + interface::{L1BatchEnv, L2BlockEnv, OneshotEnv, StoredL2BlockEnv, SystemEnv}, + utils::get_eth_call_gas_limit, + vm_latest::constants::BATCH_COMPUTATIONAL_GAS_LIMIT, }; use zksync_state::PostgresStorage; use zksync_system_constants::{ @@ -30,18 +23,15 @@ use zksync_system_constants::{ }; use zksync_types::{ api, - block::{pack_block_info, unpack_block_info, L2BlockHasher}, + block::{unpack_block_info, L2BlockHasher}, fee_model::BatchFeeInput, - get_nonce_key, - utils::{decompose_full_nonce, nonces_to_full_nonce, storage_key_for_eth_balance}, - AccountTreeId, L1BatchNumber, L2BlockNumber, Nonce, ProtocolVersionId, StorageKey, Transaction, - H256, U256, + AccountTreeId, L1BatchNumber, L2BlockNumber, ProtocolVersionId, StorageKey, H256, U256, }; -use zksync_utils::{h256_to_u256, time::seconds_since_epoch, u256_to_h256}; +use zksync_utils::{h256_to_u256, time::seconds_since_epoch}; use super::{ - vm_metrics::{self, SandboxStage, SANDBOX_METRICS}, - ApiTracer, BlockArgs, OneshotExecutor, TxExecutionArgs, TxSetupArgs, + vm_metrics::{SandboxStage, SANDBOX_METRICS}, + BlockArgs, TxSetupArgs, }; pub(super) async fn prepare_env_and_storage( @@ -207,218 +197,6 @@ fn prepare_env( (system_env, l1_batch_env) } -// public for testing purposes -#[derive(Debug)] -pub(super) struct VmSandbox { - vm: Box>, - storage_view: StoragePtr>, - transaction: Transaction, -} - -impl VmSandbox { - /// This method is blocking. - pub fn new(storage: S, mut env: OneshotEnv, execution_args: TxExecutionArgs) -> Self { - let mut storage_view = StorageView::new(storage); - Self::setup_storage_view(&mut storage_view, &execution_args, env.current_block); - - 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 = storage_view.to_rc_ptr(); - let vm = Box::new(VmInstance::new_with_specific_version( - env.l1_batch, - env.system, - storage_view.clone(), - protocol_version.into_api_vm_version(), - )); - - Self { - vm, - storage_view, - transaction: execution_args.transaction, - } - } - - /// This method is blocking. - fn setup_storage_view( - storage_view: &mut StorageView, - 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 (_, 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)); - } - - 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)); - current_balance += execution_args.added_balance; - storage_view.set_value(balance_key, u256_to_h256(current_balance)); - - // Reset L2 block info if necessary. - if let Some(current_block) = current_block { - let l2_block_info_key = StorageKey::new( - AccountTreeId::new(SYSTEM_CONTEXT_ADDRESS), - SYSTEM_CONTEXT_CURRENT_L2_BLOCK_INFO_POSITION, - ); - 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)); - - 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( - l2_block_txs_rolling_hash_key, - current_block.txs_rolling_hash, - ); - } - - let storage_view_setup_time = storage_view_setup_started_at.elapsed(); - // We don't want to emit too many logs. - if storage_view_setup_time > Duration::from_millis(10) { - tracing::debug!("Prepared the storage view (took {storage_view_setup_time:?})",); - } - } - - fn wrap_tracers( - tracers: Vec, - env: &OneshotEnv, - missed_storage_invocation_limit: usize, - ) -> Vec, HistoryDisabled>> { - let storage_invocation_tracer = StorageInvocations::new(missed_storage_invocation_limit); - let protocol_version = env.system.version; - tracers - .into_iter() - .map(|tracer| tracer.into_boxed(protocol_version)) - .chain([storage_invocation_tracer.into_tracer_pointer()]) - .collect() - } - - pub(super) fn apply(mut self, apply_fn: F) -> T - where - F: FnOnce(&mut VmInstance, Transaction) -> T, - { - let tx_id = format!( - "{:?}-{}", - self.transaction.initiator_account(), - self.transaction.nonce().unwrap_or(Nonce(0)) - ); - - let execution_latency = SANDBOX_METRICS.sandbox[&SandboxStage::Execution].start(); - let result = apply_fn(&mut *self.vm, self.transaction); - let vm_execution_took = execution_latency.observe(); - - let memory_metrics = self.vm.record_vm_memory_metrics(); - vm_metrics::report_vm_memory_metrics( - &tx_id, - &memory_metrics, - vm_execution_took, - self.storage_view.as_ref().borrow_mut().metrics(), - ); - result - } -} - -/// Main [`OneshotExecutor`] implementation used by the API server. -#[derive(Debug, Default)] -pub struct MainOneshotExecutor { - missed_storage_invocation_limit: usize, -} - -impl MainOneshotExecutor { - /// Creates a new executor with the specified limit of cache misses for storage read operations (an anti-DoS measure). - /// The limit is applied for calls and gas estimations, but not during transaction validation. - pub fn new(missed_storage_invocation_limit: usize) -> Self { - Self { - missed_storage_invocation_limit, - } - } -} - -#[async_trait] -impl OneshotExecutor for MainOneshotExecutor -where - S: ReadStorage + Send + 'static, -{ - type Tracers = Vec; - - async fn inspect_transaction( - &self, - storage: S, - env: OneshotEnv, - args: TxExecutionArgs, - tracers: Self::Tracers, - ) -> anyhow::Result { - let missed_storage_invocation_limit = match env.system.execution_mode { - // storage accesses are not limited for tx validation - TxExecutionMode::VerifyExecute => usize::MAX, - TxExecutionMode::EthCall | TxExecutionMode::EstimateFee => { - self.missed_storage_invocation_limit - } - }; - - tokio::task::spawn_blocking(move || { - let tracers = VmSandbox::wrap_tracers(tracers, &env, missed_storage_invocation_limit); - let executor = VmSandbox::new(storage, env, args); - executor.apply(|vm, transaction| { - vm.push_transaction(transaction); - vm.inspect(tracers.into(), VmExecutionMode::OneTx) - }) - }) - .await - .context("VM execution panicked") - } - - async fn inspect_transaction_with_bytecode_compression( - &self, - storage: S, - env: OneshotEnv, - args: TxExecutionArgs, - tracers: Self::Tracers, - ) -> anyhow::Result<( - Result<(), BytecodeCompressionError>, - VmExecutionResultAndLogs, - )> { - let missed_storage_invocation_limit = match env.system.execution_mode { - // storage accesses are not limited for tx validation - TxExecutionMode::VerifyExecute => usize::MAX, - TxExecutionMode::EthCall | TxExecutionMode::EstimateFee => { - self.missed_storage_invocation_limit - } - }; - - tokio::task::spawn_blocking(move || { - let tracers = VmSandbox::wrap_tracers(tracers, &env, missed_storage_invocation_limit); - let executor = VmSandbox::new(storage, env, args); - executor.apply(|vm, transaction| { - let (bytecodes_result, exec_result) = vm - .inspect_transaction_with_bytecode_compression( - tracers.into(), - transaction, - true, - ); - (bytecodes_result.map(drop), exec_result) - }) - }) - .await - .context("VM execution panicked") - } -} - async fn read_stored_l2_block( connection: &mut Connection<'_, Core>, l2_block_number: L2BlockNumber, diff --git a/core/node/api_server/src/execution_sandbox/execute.rs b/core/node/api_server/src/execution_sandbox/execute.rs index 086a75c81de..8b6203751c9 100644 --- a/core/node/api_server/src/execution_sandbox/execute.rs +++ b/core/node/api_server/src/execution_sandbox/execute.rs @@ -3,81 +3,13 @@ use async_trait::async_trait; use zksync_dal::{Connection, Core}; use zksync_multivm::interface::{ - storage::ReadStorage, BytecodeCompressionError, OneshotEnv, TransactionExecutionMetrics, - VmExecutionResultAndLogs, -}; -use zksync_types::{ - api::state_override::StateOverride, l2::L2Tx, ExecuteTransactionCommon, Nonce, - PackedEthSignature, Transaction, U256, -}; - -use super::{ - apply::{self, MainOneshotExecutor}, - storage::StorageWithOverrides, - testonly::MockOneshotExecutor, - vm_metrics, ApiTracer, BlockArgs, OneshotExecutor, TxSetupArgs, VmPermit, + executor::OneshotExecutor, storage::ReadStorage, BytecodeCompressionError, OneshotEnv, + TransactionExecutionMetrics, TxExecutionArgs, VmExecutionResultAndLogs, }; +use zksync_types::api::state_override::StateOverride; +use zksync_vm_executor::oneshot::{ApiTracer, MainOneshotExecutor, MockOneshotExecutor}; -/// Executor-independent arguments necessary to for oneshot transaction execution. -/// -/// # Developer guidelines -/// -/// Please don't add fields that duplicate `SystemEnv` or `L1BatchEnv` information, since both of these -/// are also provided to an executor. -#[derive(Debug)] -pub(crate) struct TxExecutionArgs { - /// Transaction / call itself. - pub transaction: Transaction, - /// Nonce override for the initiator account. - pub enforced_nonce: Option, - /// Balance added to the initiator account. - pub added_balance: U256, - /// If `true`, then the batch's L1 / pubdata gas price will be adjusted so that the transaction's gas per pubdata limit is <= - /// to the one in the block. This is often helpful in case we want the transaction validation to work regardless of the - /// current L1 prices for gas or pubdata. - pub adjust_pubdata_price: bool, -} - -impl TxExecutionArgs { - pub fn for_validation(tx: L2Tx) -> Self { - Self { - enforced_nonce: Some(tx.nonce()), - added_balance: U256::zero(), - adjust_pubdata_price: true, - transaction: tx.into(), - } - } - - pub fn for_eth_call(mut call: L2Tx) -> Self { - if call.common_data.signature.is_empty() { - call.common_data.signature = PackedEthSignature::default().serialize_packed().into(); - } - - Self { - enforced_nonce: None, - added_balance: U256::zero(), - adjust_pubdata_price: false, - transaction: call.into(), - } - } - - pub fn for_gas_estimate(transaction: Transaction) -> Self { - // For L2 transactions we need to explicitly put enough balance into the account of the users - // while for L1->L2 transactions the `to_mint` field plays this role - let added_balance = match &transaction.common_data { - ExecuteTransactionCommon::L2(data) => data.fee.gas_limit * data.fee.max_fee_per_gas, - ExecuteTransactionCommon::L1(_) => U256::zero(), - ExecuteTransactionCommon::ProtocolUpgrade(_) => U256::zero(), - }; - - Self { - enforced_nonce: transaction.nonce(), - added_balance, - adjust_pubdata_price: true, - transaction, - } - } -} +use super::{apply, storage::StorageWithOverrides, vm_metrics, BlockArgs, TxSetupArgs, VmPermit}; #[derive(Debug, Clone)] pub(crate) struct TransactionExecutionOutput { @@ -137,6 +69,12 @@ impl TransactionExecutor { } } +impl From for TransactionExecutor { + fn from(executor: MockOneshotExecutor) -> Self { + Self::Mock(executor) + } +} + #[async_trait] impl OneshotExecutor for TransactionExecutor where diff --git a/core/node/api_server/src/execution_sandbox/mod.rs b/core/node/api_server/src/execution_sandbox/mod.rs index f2a3f0e5f8c..3be3735c36d 100644 --- a/core/node/api_server/src/execution_sandbox/mod.rs +++ b/core/node/api_server/src/execution_sandbox/mod.rs @@ -4,13 +4,9 @@ use std::{ }; use anyhow::Context as _; -use async_trait::async_trait; use rand::{thread_rng, Rng}; use zksync_dal::{pruning_dal::PruningInfo, Connection, Core, CoreDal, DalError}; -use zksync_multivm::interface::{ - storage::ReadStorage, BytecodeCompressionError, OneshotEnv, TxExecutionMode, - VmExecutionResultAndLogs, -}; +use zksync_multivm::interface::TxExecutionMode; use zksync_state::PostgresStorageCaches; use zksync_types::{ api, fee_model::BatchFeeInput, AccountTreeId, Address, L1BatchNumber, L2BlockNumber, L2ChainId, @@ -19,8 +15,7 @@ use zksync_types::{ use self::vm_metrics::SandboxStage; pub(super) use self::{ error::SandboxExecutionError, - execute::{TransactionExecutor, TxExecutionArgs}, - tracers::ApiTracer, + execute::TransactionExecutor, validate::ValidationError, vm_metrics::{SubmitTxStage, SANDBOX_METRICS}, }; @@ -31,10 +26,8 @@ mod apply; mod error; mod execute; mod storage; -pub mod testonly; #[cfg(test)] mod tests; -mod tracers; mod validate; mod vm_metrics; @@ -170,26 +163,6 @@ pub(crate) struct TxSetupArgs { pub enforced_base_fee: Option, } -impl TxSetupArgs { - #[cfg(test)] - pub fn mock( - execution_mode: TxExecutionMode, - base_system_contracts: MultiVMBaseSystemContracts, - ) -> Self { - Self { - execution_mode, - operator_account: AccountTreeId::default(), - fee_input: BatchFeeInput::l1_pegged(55, 555), - base_system_contracts, - caches: PostgresStorageCaches::new(1, 1), - validation_computational_gas_limit: u32::MAX, - chain_id: L2ChainId::default(), - whitelisted_tokens_for_aa: Vec::new(), - enforced_base_fee: None, - } - } -} - #[derive(Debug, Clone, Copy)] struct BlockStartInfoInner { info: PruningInfo, @@ -417,28 +390,3 @@ impl BlockArgs { ) } } - -/// VM executor capable of executing isolated transactions / calls (as opposed to batch execution). -#[async_trait] -trait OneshotExecutor { - type Tracers: Default; - - async fn inspect_transaction( - &self, - storage: S, - env: OneshotEnv, - args: TxExecutionArgs, - tracers: Self::Tracers, - ) -> anyhow::Result; - - async fn inspect_transaction_with_bytecode_compression( - &self, - storage: S, - env: OneshotEnv, - args: TxExecutionArgs, - tracers: Self::Tracers, - ) -> anyhow::Result<( - Result<(), BytecodeCompressionError>, - VmExecutionResultAndLogs, - )>; -} diff --git a/core/node/api_server/src/execution_sandbox/tests.rs b/core/node/api_server/src/execution_sandbox/tests.rs index da593292e2e..b5602b3c48f 100644 --- a/core/node/api_server/src/execution_sandbox/tests.rs +++ b/core/node/api_server/src/execution_sandbox/tests.rs @@ -3,14 +3,9 @@ use assert_matches::assert_matches; use zksync_dal::ConnectionPool; use zksync_node_genesis::{insert_genesis_batch, GenesisParams}; -use zksync_node_test_utils::{create_l2_block, create_l2_transaction, prepare_recovery_snapshot}; -use zksync_types::{api::state_override::StateOverride, Transaction}; +use zksync_node_test_utils::{create_l2_block, prepare_recovery_snapshot}; use super::*; -use crate::{ - execution_sandbox::{apply::VmSandbox, storage::StorageWithOverrides}, - tx_sender::ApiContracts, -}; #[tokio::test] async fn creating_block_args() { @@ -166,6 +161,7 @@ async fn creating_block_args_after_snapshot_recovery() { } } +/* FIXME: move to executor #[tokio::test] async fn instantiating_vm() { let pool = ConnectionPool::::test_pool().await; @@ -209,3 +205,4 @@ async fn test_instantiating_vm(connection: Connection<'static, Core>, block_args .await .expect("VM execution panicked") } +*/ diff --git a/core/node/api_server/src/execution_sandbox/validate.rs b/core/node/api_server/src/execution_sandbox/validate.rs index a95cf6c3a91..3c3dca501ca 100644 --- a/core/node/api_server/src/execution_sandbox/validate.rs +++ b/core/node/api_server/src/execution_sandbox/validate.rs @@ -4,20 +4,21 @@ use anyhow::Context as _; use tracing::Instrument; use zksync_dal::{Connection, Core, CoreDal}; use zksync_multivm::{ - interface::ExecutionResult, + interface::{executor::OneshotExecutor, ExecutionResult, TxExecutionArgs}, tracers::{ValidationError as RawValidationError, ValidationTracerParams}, }; use zksync_types::{ api::state_override::StateOverride, l2::L2Tx, Address, TRUSTED_ADDRESS_SLOTS, TRUSTED_TOKEN_SLOTS, }; +use zksync_vm_executor::oneshot::ApiTracer; use super::{ apply, execute::TransactionExecutor, storage::StorageWithOverrides, vm_metrics::{SandboxStage, EXECUTION_METRICS, SANDBOX_METRICS}, - ApiTracer, BlockArgs, OneshotExecutor, TxExecutionArgs, TxSetupArgs, VmPermit, + BlockArgs, TxSetupArgs, VmPermit, }; /// Validation error used by the sandbox. Besides validation errors returned by VM, it also includes an internal error diff --git a/core/node/api_server/src/execution_sandbox/vm_metrics.rs b/core/node/api_server/src/execution_sandbox/vm_metrics.rs index ffe87be899b..89a311fbb85 100644 --- a/core/node/api_server/src/execution_sandbox/vm_metrics.rs +++ b/core/node/api_server/src/execution_sandbox/vm_metrics.rs @@ -4,77 +4,14 @@ use vise::{ Buckets, EncodeLabelSet, EncodeLabelValue, Family, Gauge, Histogram, LatencyObserver, Metrics, }; use zksync_multivm::{ - interface::{ - storage::StorageViewMetrics, TransactionExecutionMetrics, VmEvent, - VmExecutionResultAndLogs, VmMemoryMetrics, - }, + interface::{TransactionExecutionMetrics, VmEvent, VmExecutionResultAndLogs}, utils::StorageWritesDeduplicator, }; -use zksync_shared_metrics::InteractionType; use zksync_types::H256; use zksync_utils::bytecode::bytecode_len_in_bytes; use crate::utils::ReportFilter; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, EncodeLabelValue, EncodeLabelSet)] -#[metrics(label = "type", rename_all = "snake_case")] -enum SizeType { - Inner, - History, -} - -const MEMORY_SIZE_BUCKETS: Buckets = Buckets::values(&[ - 1_000.0, - 10_000.0, - 100_000.0, - 500_000.0, - 1_000_000.0, - 5_000_000.0, - 10_000_000.0, - 50_000_000.0, - 100_000_000.0, - 500_000_000.0, - 1_000_000_000.0, -]); - -#[derive(Debug, Metrics)] -#[metrics(prefix = "runtime_context_memory")] -struct RuntimeContextMemoryMetrics { - #[metrics(buckets = MEMORY_SIZE_BUCKETS)] - event_sink_size: Family>, - #[metrics(buckets = MEMORY_SIZE_BUCKETS)] - memory_size: Family>, - #[metrics(buckets = MEMORY_SIZE_BUCKETS)] - decommitter_size: Family>, - #[metrics(buckets = MEMORY_SIZE_BUCKETS)] - storage_size: Family>, - #[metrics(buckets = MEMORY_SIZE_BUCKETS)] - storage_view_cache_size: Histogram, - #[metrics(buckets = MEMORY_SIZE_BUCKETS)] - full: Histogram, -} - -#[vise::register] -static MEMORY_METRICS: vise::Global = vise::Global::new(); - -const INTERACTION_AMOUNT_BUCKETS: Buckets = Buckets::exponential(10.0..=10_000_000.0, 10.0); - -#[derive(Debug, Metrics)] -#[metrics(prefix = "runtime_context_storage_interaction")] -struct RuntimeContextStorageMetrics { - #[metrics(buckets = INTERACTION_AMOUNT_BUCKETS)] - amount: Family>, - #[metrics(buckets = Buckets::LATENCIES)] - duration: Family>, - #[metrics(buckets = Buckets::LATENCIES)] - duration_per_unit: Family>, - #[metrics(buckets = Buckets::ZERO_TO_ONE)] - ratio: Histogram, -} - -#[vise::register] -static STORAGE_METRICS: vise::Global = vise::Global::new(); - #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, EncodeLabelValue, EncodeLabelSet)] #[metrics(label = "stage", rename_all = "snake_case")] pub(super) enum SandboxStage { @@ -82,7 +19,6 @@ pub(super) enum SandboxStage { Initialization, ValidateInSandbox, Validation, - Execution, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, EncodeLabelValue, EncodeLabelSet)] @@ -185,84 +121,6 @@ pub(super) struct ExecutionMetrics { #[vise::register] pub(super) static EXECUTION_METRICS: vise::Global = vise::Global::new(); -pub(super) fn report_vm_memory_metrics( - tx_id: &str, - memory_metrics: &VmMemoryMetrics, - vm_execution_took: Duration, - storage_metrics: StorageViewMetrics, -) { - 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); - MEMORY_METRICS.memory_size[&SizeType::Inner].observe(memory_metrics.memory_inner); - MEMORY_METRICS.memory_size[&SizeType::History].observe(memory_metrics.memory_history); - MEMORY_METRICS.decommitter_size[&SizeType::Inner] - .observe(memory_metrics.decommittment_processor_inner); - MEMORY_METRICS.decommitter_size[&SizeType::History] - .observe(memory_metrics.decommittment_processor_history); - MEMORY_METRICS.storage_size[&SizeType::Inner].observe(memory_metrics.storage_inner); - MEMORY_METRICS.storage_size[&SizeType::History].observe(memory_metrics.storage_history); - - MEMORY_METRICS - .storage_view_cache_size - .observe(storage_metrics.cache_size); - MEMORY_METRICS - .full - .observe(memory_metrics.full_size() + storage_metrics.cache_size); - - let total_storage_invocations = storage_metrics.get_value_storage_invocations - + storage_metrics.set_value_storage_invocations; - let total_time_spent_in_storage = - storage_metrics.time_spent_on_get_value + storage_metrics.time_spent_on_set_value; - - STORAGE_METRICS.amount[&InteractionType::Missed] - .observe(storage_metrics.storage_invocations_missed); - STORAGE_METRICS.amount[&InteractionType::GetValue] - .observe(storage_metrics.get_value_storage_invocations); - STORAGE_METRICS.amount[&InteractionType::SetValue] - .observe(storage_metrics.set_value_storage_invocations); - STORAGE_METRICS.amount[&InteractionType::Total].observe(total_storage_invocations); - - STORAGE_METRICS.duration[&InteractionType::Missed] - .observe(storage_metrics.time_spent_on_storage_missed); - STORAGE_METRICS.duration[&InteractionType::GetValue] - .observe(storage_metrics.time_spent_on_get_value); - STORAGE_METRICS.duration[&InteractionType::SetValue] - .observe(storage_metrics.time_spent_on_set_value); - STORAGE_METRICS.duration[&InteractionType::Total].observe(total_time_spent_in_storage); - - if total_storage_invocations > 0 { - STORAGE_METRICS.duration_per_unit[&InteractionType::Total] - .observe(total_time_spent_in_storage.div_f64(total_storage_invocations as f64)); - } - if storage_metrics.storage_invocations_missed > 0 { - let duration_per_unit = storage_metrics - .time_spent_on_storage_missed - .div_f64(storage_metrics.storage_invocations_missed as f64); - STORAGE_METRICS.duration_per_unit[&InteractionType::Missed].observe(duration_per_unit); - } - - STORAGE_METRICS - .ratio - .observe(total_time_spent_in_storage.as_secs_f64() / vm_execution_took.as_secs_f64()); - - const STORAGE_INVOCATIONS_DEBUG_THRESHOLD: usize = 1_000; - - if total_storage_invocations > STORAGE_INVOCATIONS_DEBUG_THRESHOLD { - tracing::info!( - "Tx {tx_id} resulted in {total_storage_invocations} storage_invocations, {} new_storage_invocations, \ - {} get_value_storage_invocations, {} set_value_storage_invocations, \ - vm execution took {vm_execution_took:?}, storage interaction took {total_time_spent_in_storage:?} \ - (missed: {:?} get: {:?} set: {:?})", - storage_metrics.storage_invocations_missed, - storage_metrics.get_value_storage_invocations, - storage_metrics.set_value_storage_invocations, - storage_metrics.time_spent_on_storage_missed, - storage_metrics.time_spent_on_get_value, - storage_metrics.time_spent_on_set_value, - ); - } -} - pub(super) fn collect_tx_execution_metrics( contracts_deployed: u16, result: &VmExecutionResultAndLogs, diff --git a/core/node/api_server/src/tx_sender/mod.rs b/core/node/api_server/src/tx_sender/mod.rs index 5f913e305cd..c01705669f3 100644 --- a/core/node/api_server/src/tx_sender/mod.rs +++ b/core/node/api_server/src/tx_sender/mod.rs @@ -10,7 +10,9 @@ use zksync_dal::{ transactions_dal::L2TxSubmissionResult, Connection, ConnectionPool, Core, CoreDal, }; use zksync_multivm::{ - interface::{TransactionExecutionMetrics, TxExecutionMode, VmExecutionResultAndLogs}, + interface::{ + TransactionExecutionMetrics, TxExecutionArgs, TxExecutionMode, VmExecutionResultAndLogs, + }, utils::{ adjust_pubdata_price_for_tx, derive_base_fee_and_gas_per_pubdata, derive_overhead, get_max_batch_gas_limit, @@ -41,8 +43,8 @@ pub(super) use self::result::SubmitTxError; use self::{master_pool_sink::MasterPoolSink, tx_sink::TxSink}; use crate::{ execution_sandbox::{ - BlockArgs, SubmitTxStage, TransactionExecutor, TxExecutionArgs, TxSetupArgs, - VmConcurrencyBarrier, VmConcurrencyLimiter, VmPermit, SANDBOX_METRICS, + BlockArgs, SubmitTxStage, TransactionExecutor, TxSetupArgs, VmConcurrencyBarrier, + VmConcurrencyLimiter, VmPermit, SANDBOX_METRICS, }, tx_sender::result::ApiCallResult, }; diff --git a/core/node/api_server/src/tx_sender/tests.rs b/core/node/api_server/src/tx_sender/tests.rs index 5b2ab0495da..0ac3eb0b4f3 100644 --- a/core/node/api_server/src/tx_sender/tests.rs +++ b/core/node/api_server/src/tx_sender/tests.rs @@ -9,12 +9,10 @@ use zksync_node_genesis::{insert_genesis_batch, GenesisParams}; use zksync_node_test_utils::{create_l2_block, create_l2_transaction, prepare_recovery_snapshot}; use zksync_types::{api, get_nonce_key, L1BatchNumber, L2BlockNumber, StorageLog}; use zksync_utils::u256_to_h256; +use zksync_vm_executor::oneshot::MockOneshotExecutor; use super::*; -use crate::{ - execution_sandbox::{testonly::MockOneshotExecutor, BlockStartInfo}, - web3::testonly::create_test_tx_sender, -}; +use crate::{execution_sandbox::BlockStartInfo, web3::testonly::create_test_tx_sender}; #[tokio::test] async fn getting_nonce_for_account() { diff --git a/core/node/api_server/src/web3/namespaces/debug.rs b/core/node/api_server/src/web3/namespaces/debug.rs index 473391476a3..053fb338324 100644 --- a/core/node/api_server/src/web3/namespaces/debug.rs +++ b/core/node/api_server/src/web3/namespaces/debug.rs @@ -4,7 +4,7 @@ use anyhow::Context as _; use once_cell::sync::OnceCell; use zksync_dal::{CoreDal, DalError}; use zksync_multivm::{ - interface::{Call, CallType, ExecutionResult, TxExecutionMode}, + interface::{Call, CallType, ExecutionResult, TxExecutionArgs, TxExecutionMode}, vm_latest::constants::BATCH_COMPUTATIONAL_GAS_LIMIT, }; use zksync_system_constants::MAX_ENCODED_TX_SIZE; @@ -16,10 +16,11 @@ use zksync_types::{ transaction_request::CallRequest, web3, AccountTreeId, H256, U256, }; +use zksync_vm_executor::oneshot::ApiTracer; use zksync_web3_decl::error::Web3Error; use crate::{ - execution_sandbox::{ApiTracer, TxExecutionArgs, TxSetupArgs}, + execution_sandbox::TxSetupArgs, tx_sender::{ApiContracts, TxSenderConfig}, web3::{backend_jsonrpsee::MethodTracer, state::RpcState}, }; diff --git a/core/node/api_server/src/web3/testonly.rs b/core/node/api_server/src/web3/testonly.rs index 9f6b30b6026..a77498d4341 100644 --- a/core/node/api_server/src/web3/testonly.rs +++ b/core/node/api_server/src/web3/testonly.rs @@ -14,12 +14,10 @@ use zksync_types::{ fee_model::{BatchFeeInput, FeeParams}, L2ChainId, }; +use zksync_vm_executor::oneshot::MockOneshotExecutor; use super::{metrics::ApiTransportLabel, *}; -use crate::{ - execution_sandbox::{testonly::MockOneshotExecutor, TransactionExecutor}, - tx_sender::TxSenderConfig, -}; +use crate::{execution_sandbox::TransactionExecutor, tx_sender::TxSenderConfig}; const TEST_TIMEOUT: Duration = Duration::from_secs(90); const POLL_INTERVAL: Duration = Duration::from_millis(50); diff --git a/core/node/api_server/src/web3/tests/mod.rs b/core/node/api_server/src/web3/tests/mod.rs index 5617b097c0c..635620e9c52 100644 --- a/core/node/api_server/src/web3/tests/mod.rs +++ b/core/node/api_server/src/web3/tests/mod.rs @@ -42,6 +42,7 @@ use zksync_types::{ U256, U64, }; use zksync_utils::u256_to_h256; +use zksync_vm_executor::oneshot::MockOneshotExecutor; use zksync_web3_decl::{ client::{Client, DynClient, L2}, jsonrpsee::{ @@ -57,10 +58,7 @@ use zksync_web3_decl::{ }; use super::*; -use crate::{ - execution_sandbox::testonly::MockOneshotExecutor, - web3::testonly::{spawn_http_server, spawn_ws_server}, -}; +use crate::web3::testonly::{spawn_http_server, spawn_ws_server}; mod debug; mod filters; diff --git a/core/node/api_server/src/web3/tests/vm.rs b/core/node/api_server/src/web3/tests/vm.rs index 5b04250eebf..6cd12ed8d14 100644 --- a/core/node/api_server/src/web3/tests/vm.rs +++ b/core/node/api_server/src/web3/tests/vm.rs @@ -11,6 +11,7 @@ use zksync_types::{ L2ChainId, PackedEthSignature, StorageLogKind, StorageLogWithPreviousValue, U256, }; use zksync_utils::u256_to_h256; +use zksync_vm_executor::oneshot::MockOneshotExecutor; use zksync_web3_decl::namespaces::DebugNamespaceClient; use super::*; From a2f6983b37477971cc2075b4cffeb4614838977f Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 3 Sep 2024 16:28:59 +0300 Subject: [PATCH 02/11] Sketch tracer adapter for latest VM --- core/lib/multivm/src/glue/tracers/mod.rs | 4 +- core/lib/multivm/src/tracers/mod.rs | 4 +- core/lib/multivm/src/tracers/validator/mod.rs | 7 +- .../multivm/src/tracers/validator/types.rs | 57 +------------ .../src/tracers/validator/vm_1_4_1/mod.rs | 4 +- .../src/tracers/validator/vm_1_4_2/mod.rs | 4 +- .../validator/vm_boojum_integration/mod.rs | 4 +- .../src/tracers/validator/vm_latest/mod.rs | 4 +- .../validator/vm_refunds_enhancement/mod.rs | 4 +- .../validator/vm_virtual_blocks/mod.rs | 3 +- core/lib/vm_executor/src/oneshot/mock.rs | 8 +- core/lib/vm_executor/src/oneshot/mod.rs | 49 +++++------- core/lib/vm_executor/src/oneshot/tracers.rs | 79 ++++++++++++------- core/lib/vm_interface/src/executor.rs | 8 +- core/lib/vm_interface/src/lib.rs | 4 +- core/lib/vm_interface/src/types/inputs/mod.rs | 14 ++++ core/lib/vm_interface/src/types/tracer.rs | 51 ++++++++++++ .../src/execution_sandbox/execute.rs | 20 ++--- .../src/execution_sandbox/validate.rs | 26 ++++-- core/node/api_server/src/tx_sender/mod.rs | 9 ++- .../api_server/src/web3/namespaces/debug.rs | 21 ++--- 21 files changed, 208 insertions(+), 176 deletions(-) diff --git a/core/lib/multivm/src/glue/tracers/mod.rs b/core/lib/multivm/src/glue/tracers/mod.rs index bf2f67cae50..3b9d576f32a 100644 --- a/core/lib/multivm/src/glue/tracers/mod.rs +++ b/core/lib/multivm/src/glue/tracers/mod.rs @@ -32,7 +32,7 @@ use crate::{interface::storage::WriteStorage, tracers::old::OldTracers, HistoryMode}; -pub type MultiVmTracerPointer = Box>; +pub type MultiVmTracerPointer = Box + Send>; pub trait MultiVMTracer: IntoLatestTracer @@ -45,7 +45,7 @@ pub trait MultiVMTracer: { fn into_tracer_pointer(self) -> MultiVmTracerPointer where - Self: Sized + 'static, + Self: Sized + Send + 'static, { Box::new(self) } diff --git a/core/lib/multivm/src/tracers/mod.rs b/core/lib/multivm/src/tracers/mod.rs index 69501cf3988..8dacfaad4ae 100644 --- a/core/lib/multivm/src/tracers/mod.rs +++ b/core/lib/multivm/src/tracers/mod.rs @@ -3,9 +3,7 @@ pub use self::{ multivm_dispatcher::TracerDispatcher, prestate_tracer::PrestateTracer, storage_invocation::StorageInvocations, - validator::{ - ValidationError, ValidationTracer, ValidationTracerParams, ViolatedValidationRule, - }, + validator::{ValidationError, ValidationTracer}, }; mod call_tracer; diff --git a/core/lib/multivm/src/tracers/validator/mod.rs b/core/lib/multivm/src/tracers/validator/mod.rs index 307256792cf..06129db40ae 100644 --- a/core/lib/multivm/src/tracers/validator/mod.rs +++ b/core/lib/multivm/src/tracers/validator/mod.rs @@ -10,11 +10,14 @@ use zksync_types::{ }; use zksync_utils::{be_bytes_to_safe_address, u256_to_account_address, u256_to_h256}; +pub use self::types::ValidationError; use self::types::{NewTrustedValidationItems, ValidationTracerMode}; -pub use self::types::{ValidationError, ValidationTracerParams, ViolatedValidationRule}; use crate::{ glue::tracers::IntoOldVmTracer, - interface::storage::{StoragePtr, WriteStorage}, + interface::{ + storage::{StoragePtr, WriteStorage}, + tracer::{ValidationTracerParams, ViolatedValidationRule}, + }, }; mod types; diff --git a/core/lib/multivm/src/tracers/validator/types.rs b/core/lib/multivm/src/tracers/validator/types.rs index 418d2b89350..a57dddef33e 100644 --- a/core/lib/multivm/src/tracers/validator/types.rs +++ b/core/lib/multivm/src/tracers/validator/types.rs @@ -1,9 +1,8 @@ -use std::{collections::HashSet, fmt, fmt::Display}; +use std::fmt::Display; -use zksync_types::{Address, H256, U256}; -use zksync_utils::u256_to_h256; +use zksync_types::{Address, H256}; -use crate::interface::Halt; +use crate::interface::{tracer::ViolatedValidationRule, Halt}; #[derive(Debug, Clone, Eq, PartialEq, Copy)] #[allow(clippy::enum_variant_names)] @@ -22,56 +21,6 @@ pub(super) struct NewTrustedValidationItems { pub(super) new_trusted_addresses: Vec
, } -#[derive(Debug, Clone)] -pub struct ValidationTracerParams { - pub user_address: Address, - pub paymaster_address: Address, - /// Slots that are trusted (i.e. the user can access them). - pub trusted_slots: HashSet<(Address, U256)>, - /// Trusted addresses (the user can access any slots on these addresses). - pub trusted_addresses: HashSet
, - /// Slots, that are trusted and the value of them is the new trusted address. - /// They are needed to work correctly with beacon proxy, where the address of the implementation is - /// stored in the beacon. - pub trusted_address_slots: HashSet<(Address, U256)>, - /// Number of computational gas that validation step is allowed to use. - pub computational_gas_limit: u32, -} - -#[derive(Debug, Clone)] -pub enum ViolatedValidationRule { - TouchedUnallowedStorageSlots(Address, U256), - CalledContractWithNoCode(Address), - TouchedUnallowedContext, - TookTooManyComputationalGas(u32), -} - -impl fmt::Display for ViolatedValidationRule { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - ViolatedValidationRule::TouchedUnallowedStorageSlots(contract, key) => write!( - f, - "Touched unallowed storage slots: address {}, key: {}", - hex::encode(contract), - hex::encode(u256_to_h256(*key)) - ), - ViolatedValidationRule::CalledContractWithNoCode(contract) => { - write!(f, "Called contract with no code: {}", hex::encode(contract)) - } - ViolatedValidationRule::TouchedUnallowedContext => { - write!(f, "Touched unallowed context") - } - ViolatedValidationRule::TookTooManyComputationalGas(gas_limit) => { - write!( - f, - "Took too many computational gas, allowed limit: {}", - gas_limit - ) - } - } - } -} - #[derive(Debug, Clone)] pub enum ValidationError { FailedTx(Halt), diff --git a/core/lib/multivm/src/tracers/validator/vm_1_4_1/mod.rs b/core/lib/multivm/src/tracers/validator/vm_1_4_1/mod.rs index 2beca41fb48..5c245dcb75a 100644 --- a/core/lib/multivm/src/tracers/validator/vm_1_4_1/mod.rs +++ b/core/lib/multivm/src/tracers/validator/vm_1_4_1/mod.rs @@ -9,13 +9,13 @@ use zksync_utils::{h256_to_account_address, u256_to_account_address, u256_to_h25 use crate::{ interface::{ storage::{StoragePtr, WriteStorage}, - tracer::{TracerExecutionStatus, TracerExecutionStopReason}, + tracer::{TracerExecutionStatus, TracerExecutionStopReason, ViolatedValidationRule}, Halt, }, tracers::{ dynamic::vm_1_4_1::DynTracer, validator::{ - types::{NewTrustedValidationItems, ValidationTracerMode, ViolatedValidationRule}, + types::{NewTrustedValidationItems, ValidationTracerMode}, ValidationRoundResult, ValidationTracer, }, }, diff --git a/core/lib/multivm/src/tracers/validator/vm_1_4_2/mod.rs b/core/lib/multivm/src/tracers/validator/vm_1_4_2/mod.rs index 3394a6c3f2b..edea8ddbadd 100644 --- a/core/lib/multivm/src/tracers/validator/vm_1_4_2/mod.rs +++ b/core/lib/multivm/src/tracers/validator/vm_1_4_2/mod.rs @@ -9,13 +9,13 @@ use zksync_utils::{h256_to_account_address, u256_to_account_address, u256_to_h25 use crate::{ interface::{ storage::{StoragePtr, WriteStorage}, - tracer::{TracerExecutionStatus, TracerExecutionStopReason}, + tracer::{TracerExecutionStatus, TracerExecutionStopReason, ViolatedValidationRule}, Halt, }, tracers::{ dynamic::vm_1_4_1::DynTracer, validator::{ - types::{NewTrustedValidationItems, ValidationTracerMode, ViolatedValidationRule}, + types::{NewTrustedValidationItems, ValidationTracerMode}, ValidationRoundResult, ValidationTracer, }, }, diff --git a/core/lib/multivm/src/tracers/validator/vm_boojum_integration/mod.rs b/core/lib/multivm/src/tracers/validator/vm_boojum_integration/mod.rs index 53b5bf04d2e..ef032b5310a 100644 --- a/core/lib/multivm/src/tracers/validator/vm_boojum_integration/mod.rs +++ b/core/lib/multivm/src/tracers/validator/vm_boojum_integration/mod.rs @@ -9,13 +9,13 @@ use zksync_utils::{h256_to_account_address, u256_to_account_address, u256_to_h25 use crate::{ interface::{ storage::{StoragePtr, WriteStorage}, - tracer::{TracerExecutionStatus, TracerExecutionStopReason}, + tracer::{TracerExecutionStatus, TracerExecutionStopReason, ViolatedValidationRule}, Halt, }, tracers::{ dynamic::vm_1_4_0::DynTracer, validator::{ - types::{NewTrustedValidationItems, ValidationTracerMode, ViolatedValidationRule}, + types::{NewTrustedValidationItems, ValidationTracerMode}, ValidationRoundResult, ValidationTracer, }, }, diff --git a/core/lib/multivm/src/tracers/validator/vm_latest/mod.rs b/core/lib/multivm/src/tracers/validator/vm_latest/mod.rs index e963c79f4e4..68064902000 100644 --- a/core/lib/multivm/src/tracers/validator/vm_latest/mod.rs +++ b/core/lib/multivm/src/tracers/validator/vm_latest/mod.rs @@ -9,13 +9,13 @@ use zksync_utils::{h256_to_account_address, u256_to_account_address, u256_to_h25 use crate::{ interface::{ storage::{StoragePtr, WriteStorage}, - tracer::{TracerExecutionStatus, TracerExecutionStopReason}, + tracer::{TracerExecutionStatus, TracerExecutionStopReason, ViolatedValidationRule}, Halt, }, tracers::{ dynamic::vm_1_5_0::DynTracer, validator::{ - types::{NewTrustedValidationItems, ValidationTracerMode, ViolatedValidationRule}, + types::{NewTrustedValidationItems, ValidationTracerMode}, ValidationRoundResult, ValidationTracer, }, }, diff --git a/core/lib/multivm/src/tracers/validator/vm_refunds_enhancement/mod.rs b/core/lib/multivm/src/tracers/validator/vm_refunds_enhancement/mod.rs index 6107125d14d..cd42221a42b 100644 --- a/core/lib/multivm/src/tracers/validator/vm_refunds_enhancement/mod.rs +++ b/core/lib/multivm/src/tracers/validator/vm_refunds_enhancement/mod.rs @@ -9,13 +9,13 @@ use zksync_utils::{h256_to_account_address, u256_to_account_address, u256_to_h25 use crate::{ interface::{ storage::{StoragePtr, WriteStorage}, - tracer::{TracerExecutionStatus, TracerExecutionStopReason}, + tracer::{TracerExecutionStatus, TracerExecutionStopReason, ViolatedValidationRule}, Halt, }, tracers::{ dynamic::vm_1_3_3::DynTracer, validator::{ - types::{NewTrustedValidationItems, ValidationTracerMode, ViolatedValidationRule}, + types::{NewTrustedValidationItems, ValidationTracerMode}, ValidationRoundResult, ValidationTracer, }, }, diff --git a/core/lib/multivm/src/tracers/validator/vm_virtual_blocks/mod.rs b/core/lib/multivm/src/tracers/validator/vm_virtual_blocks/mod.rs index bb166bedcda..45470422b00 100644 --- a/core/lib/multivm/src/tracers/validator/vm_virtual_blocks/mod.rs +++ b/core/lib/multivm/src/tracers/validator/vm_virtual_blocks/mod.rs @@ -9,12 +9,13 @@ use zksync_utils::{h256_to_account_address, u256_to_account_address, u256_to_h25 use crate::{ interface::{ storage::{StoragePtr, WriteStorage}, + tracer::ViolatedValidationRule, VmExecutionResultAndLogs, }, tracers::{ dynamic::vm_1_3_3::DynTracer, validator::{ - types::{NewTrustedValidationItems, ValidationTracerMode, ViolatedValidationRule}, + types::{NewTrustedValidationItems, ValidationTracerMode}, ValidationRoundResult, ValidationTracer, }, }, diff --git a/core/lib/vm_executor/src/oneshot/mock.rs b/core/lib/vm_executor/src/oneshot/mock.rs index 5e84605f99c..510a43a2ae9 100644 --- a/core/lib/vm_executor/src/oneshot/mock.rs +++ b/core/lib/vm_executor/src/oneshot/mock.rs @@ -3,7 +3,7 @@ use std::fmt; use async_trait::async_trait; use zksync_multivm::interface::{ executor::OneshotExecutor, storage::ReadStorage, BytecodeCompressionError, ExecutionResult, - OneshotEnv, TxExecutionArgs, TxExecutionMode, VmExecutionResultAndLogs, + OneshotEnv, OneshotTracers, TxExecutionArgs, TxExecutionMode, VmExecutionResultAndLogs, }; use zksync_types::Transaction; @@ -88,14 +88,12 @@ impl OneshotExecutor for MockOneshotExecutor where S: ReadStorage + Send + 'static, { - type Tracers = (); - async fn inspect_transaction( &self, _storage: S, env: OneshotEnv, args: TxExecutionArgs, - (): Self::Tracers, + _tracers: OneshotTracers<'_>, ) -> anyhow::Result { Ok(self.mock_inspect(env, args)) } @@ -105,7 +103,7 @@ where _storage: S, env: OneshotEnv, args: TxExecutionArgs, - (): Self::Tracers, + _tracers: OneshotTracers<'_>, ) -> anyhow::Result<( Result<(), BytecodeCompressionError>, VmExecutionResultAndLogs, diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs index 65a574a4e60..d075c6043e0 100644 --- a/core/lib/vm_executor/src/oneshot/mod.rs +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -8,14 +8,14 @@ use zksync_multivm::{ interface::{ executor::OneshotExecutor, storage::{ReadStorage, StoragePtr, StorageView, WriteStorage}, - BytecodeCompressionError, OneshotEnv, StoredL2BlockEnv, TxExecutionArgs, TxExecutionMode, - VmExecutionMode, VmExecutionResultAndLogs, VmInterface, + BytecodeCompressionError, OneshotEnv, OneshotTracers, StoredL2BlockEnv, TxExecutionArgs, + TxExecutionMode, VmExecutionMode, VmExecutionResultAndLogs, VmInterface, }, tracers::StorageInvocations, utils::adjust_pubdata_price_for_tx, vm_latest::HistoryDisabled, zk_evm_latest::ethereum_types::U256, - MultiVMTracer, MultiVmTracerPointer, VmInstance, + MultiVMTracer, VmInstance, }; use zksync_types::{ block::pack_block_info, @@ -26,7 +26,8 @@ use zksync_types::{ }; use zksync_utils::{h256_to_u256, u256_to_h256}; -pub use self::{mock::MockOneshotExecutor, tracers::ApiTracer}; +pub use self::mock::MockOneshotExecutor; +use self::tracers::TracersAdapter; mod metrics; mod mock; @@ -53,15 +54,15 @@ impl OneshotExecutor for MainOneshotExecutor where S: ReadStorage + Send + 'static, { - type Tracers = Vec; - async fn inspect_transaction( &self, storage: S, env: OneshotEnv, args: TxExecutionArgs, - tracers: Self::Tracers, + tracers: OneshotTracers<'_>, ) -> anyhow::Result { + let tracers = TracersAdapter::new(tracers); + let mut vm_tracers = tracers.to_boxed(env.system.version); let missed_storage_invocation_limit = match env.system.execution_mode { // storage accesses are not limited for tx validation TxExecutionMode::VerifyExecute => usize::MAX, @@ -69,17 +70,20 @@ where self.missed_storage_invocation_limit } }; + vm_tracers + .push(StorageInvocations::new(missed_storage_invocation_limit).into_tracer_pointer()); - tokio::task::spawn_blocking(move || { - let tracers = VmSandbox::wrap_tracers(tracers, &env, missed_storage_invocation_limit); + let result = tokio::task::spawn_blocking(move || { let executor = VmSandbox::new(storage, env, args); executor.apply(|vm, transaction| { vm.push_transaction(transaction); - vm.inspect(tracers.into(), VmExecutionMode::OneTx) + vm.inspect(vm_tracers.into(), VmExecutionMode::OneTx) }) }) .await - .context("VM execution panicked") + .context("VM execution panicked")?; + + Ok(result) } async fn inspect_transaction_with_bytecode_compression( @@ -87,11 +91,13 @@ where storage: S, env: OneshotEnv, args: TxExecutionArgs, - tracers: Self::Tracers, + tracers: OneshotTracers<'_>, ) -> anyhow::Result<( Result<(), BytecodeCompressionError>, VmExecutionResultAndLogs, )> { + let tracers = TracersAdapter::new(tracers); + let mut vm_tracers = tracers.to_boxed(env.system.version); let missed_storage_invocation_limit = match env.system.execution_mode { // storage accesses are not limited for tx validation TxExecutionMode::VerifyExecute => usize::MAX, @@ -99,14 +105,15 @@ where self.missed_storage_invocation_limit } }; + vm_tracers + .push(StorageInvocations::new(missed_storage_invocation_limit).into_tracer_pointer()); tokio::task::spawn_blocking(move || { - let tracers = VmSandbox::wrap_tracers(tracers, &env, missed_storage_invocation_limit); let executor = VmSandbox::new(storage, env, args); executor.apply(|vm, transaction| { let (bytecodes_result, exec_result) = vm .inspect_transaction_with_bytecode_compression( - tracers.into(), + vm_tracers.into(), transaction, true, ); @@ -204,20 +211,6 @@ impl VmSandbox { } } - fn wrap_tracers( - tracers: Vec, - env: &OneshotEnv, - missed_storage_invocation_limit: usize, - ) -> Vec, HistoryDisabled>> { - let storage_invocation_tracer = StorageInvocations::new(missed_storage_invocation_limit); - let protocol_version = env.system.version; - tracers - .into_iter() - .map(|tracer| tracer.into_boxed(protocol_version)) - .chain([storage_invocation_tracer.into_tracer_pointer()]) - .collect() - } - pub(super) fn apply(mut self, apply_fn: F) -> T where F: FnOnce(&mut VmInstance, Transaction) -> T, diff --git a/core/lib/vm_executor/src/oneshot/tracers.rs b/core/lib/vm_executor/src/oneshot/tracers.rs index 6fdc3dbc7b6..fae9df45648 100644 --- a/core/lib/vm_executor/src/oneshot/tracers.rs +++ b/core/lib/vm_executor/src/oneshot/tracers.rs @@ -1,9 +1,9 @@ -use std::sync::Arc; +use std::{sync::Arc, thread}; use once_cell::sync::OnceCell; use zksync_multivm::{ - interface::{storage::WriteStorage, Call}, - tracers::{CallTracer, ValidationTracer, ValidationTracerParams, ViolatedValidationRule}, + interface::{storage::WriteStorage, tracer::ViolatedValidationRule, Call, OneshotTracers}, + tracers::{CallTracer, ValidationTracer}, vm_latest::HistoryDisabled, MultiVMTracer, MultiVmTracerPointer, }; @@ -11,40 +11,61 @@ use zksync_types::ProtocolVersionId; /// Custom tracers supported by the API sandbox. #[derive(Debug)] -pub enum ApiTracer { - CallTracer(Arc>>), - Validation { - params: ValidationTracerParams, - result: Arc>, - }, +pub(super) struct TracersAdapter<'a> { + inner: OneshotTracers<'a>, + calls_cell: Arc>>, + validation_cell: Arc>, } -impl ApiTracer { - pub fn validation( - params: ValidationTracerParams, - ) -> (Self, Arc>) { - let result = Arc::>::default(); - let this = Self::Validation { - params, - result: result.clone(), - }; - (this, result) +impl<'a> TracersAdapter<'a> { + pub fn new(inner: OneshotTracers<'a>) -> Self { + Self { + inner, + calls_cell: Arc::default(), + validation_cell: Arc::default(), + } } - pub(super) fn into_boxed( - self, + pub fn to_boxed( + &self, protocol_version: ProtocolVersionId, - ) -> MultiVmTracerPointer + ) -> Vec> where S: WriteStorage, { - match self { - Self::CallTracer(traces) => CallTracer::new(traces).into_tracer_pointer(), - Self::Validation { params, result } => { - let (mut tracer, _) = - ValidationTracer::::new(params, protocol_version.into()); - tracer.result = result; - tracer.into_tracer_pointer() + match self.inner { + OneshotTracers::None => vec![], + OneshotTracers::Calls(_) => { + vec![CallTracer::new(self.calls_cell.clone()).into_tracer_pointer()] + } + OneshotTracers::Validation { params, .. } => { + let (mut tracer, _) = ValidationTracer::::new( + params.clone(), + protocol_version.into(), + ); + tracer.result = self.validation_cell.clone(); + vec![tracer.into_tracer_pointer()] + } + } + } +} + +impl Drop for TracersAdapter<'_> { + fn drop(&mut self) { + if thread::panicking() { + return; + } + + match &mut self.inner { + OneshotTracers::None => { /* do nothing */ } + OneshotTracers::Calls(calls) => { + **calls = self.calls_cell.get().cloned().unwrap_or_default(); + } + OneshotTracers::Validation { result, .. } => { + **result = match self.validation_cell.get() { + Some(rule) => Err(rule.clone()), + None => Ok(()), + }; } } } diff --git a/core/lib/vm_interface/src/executor.rs b/core/lib/vm_interface/src/executor.rs index f6aad29387c..d07414791c2 100644 --- a/core/lib/vm_interface/src/executor.rs +++ b/core/lib/vm_interface/src/executor.rs @@ -8,7 +8,7 @@ use zksync_types::Transaction; use crate::{ storage::{ReadStorage, StorageView}, BatchTransactionExecutionResult, BytecodeCompressionError, FinishedL1Batch, L1BatchEnv, - L2BlockEnv, OneshotEnv, SystemEnv, TxExecutionArgs, VmExecutionResultAndLogs, + L2BlockEnv, OneshotEnv, OneshotTracers, SystemEnv, TxExecutionArgs, VmExecutionResultAndLogs, }; /// Factory of [`BatchExecutor`]s. @@ -47,14 +47,12 @@ pub trait BatchExecutor: 'static + Send + fmt::Debug { /// VM executor capable of executing isolated transactions / calls (as opposed to [batch execution](BatchExecutor)). #[async_trait] pub trait OneshotExecutor { - type Tracers: Default; // FIXME: revise - async fn inspect_transaction( &self, storage: S, env: OneshotEnv, args: TxExecutionArgs, - tracers: Self::Tracers, + tracers: OneshotTracers<'_>, ) -> anyhow::Result; async fn inspect_transaction_with_bytecode_compression( @@ -62,7 +60,7 @@ pub trait OneshotExecutor { storage: S, env: OneshotEnv, args: TxExecutionArgs, - tracers: Self::Tracers, + tracers: OneshotTracers<'_>, ) -> anyhow::Result<( Result<(), BytecodeCompressionError>, VmExecutionResultAndLogs, diff --git a/core/lib/vm_interface/src/lib.rs b/core/lib/vm_interface/src/lib.rs index 333147b4fd7..24cdb0936bb 100644 --- a/core/lib/vm_interface/src/lib.rs +++ b/core/lib/vm_interface/src/lib.rs @@ -24,8 +24,8 @@ pub use crate::{ VmRevertReason, VmRevertReasonParsingError, }, inputs::{ - L1BatchEnv, L2BlockEnv, OneshotEnv, StoredL2BlockEnv, SystemEnv, TxExecutionArgs, - TxExecutionMode, VmExecutionMode, + L1BatchEnv, L2BlockEnv, OneshotEnv, OneshotTracers, StoredL2BlockEnv, SystemEnv, + TxExecutionArgs, TxExecutionMode, VmExecutionMode, }, outputs::{ BatchTransactionExecutionResult, BootloaderMemory, Call, CallType, CircuitStatistic, diff --git a/core/lib/vm_interface/src/types/inputs/mod.rs b/core/lib/vm_interface/src/types/inputs/mod.rs index 871421ac344..1f738a9d30c 100644 --- a/core/lib/vm_interface/src/types/inputs/mod.rs +++ b/core/lib/vm_interface/src/types/inputs/mod.rs @@ -8,6 +8,10 @@ pub use self::{ l2_block::{L2BlockEnv, StoredL2BlockEnv}, system_env::{SystemEnv, TxExecutionMode}, }; +use crate::{ + tracer::{ValidationTracerParams, ViolatedValidationRule}, + Call, +}; mod execution_mode; mod l1_batch_env; @@ -86,3 +90,13 @@ impl TxExecutionArgs { } } } + +#[derive(Debug)] +pub enum OneshotTracers<'a> { + None, + Calls(&'a mut Vec), + Validation { + params: &'a ValidationTracerParams, + result: &'a mut Result<(), ViolatedValidationRule>, + }, +} diff --git a/core/lib/vm_interface/src/types/tracer.rs b/core/lib/vm_interface/src/types/tracer.rs index 1b42b2eabbb..dc83287cd51 100644 --- a/core/lib/vm_interface/src/types/tracer.rs +++ b/core/lib/vm_interface/src/types/tracer.rs @@ -1,3 +1,7 @@ +use std::{collections::HashSet, fmt}; + +use zksync_types::{Address, U256}; + use crate::Halt; #[derive(Debug, Clone, PartialEq)] @@ -37,3 +41,50 @@ pub enum VmExecutionStopReason { VmFinished, TracerRequestedStop(TracerExecutionStopReason), } + +#[derive(Debug, Clone)] +pub struct ValidationTracerParams { + pub user_address: Address, + pub paymaster_address: Address, + /// Slots that are trusted (i.e. the user can access them). + pub trusted_slots: HashSet<(Address, U256)>, + /// Trusted addresses (the user can access any slots on these addresses). + pub trusted_addresses: HashSet
, + /// Slots, that are trusted and the value of them is the new trusted address. + /// They are needed to work correctly with beacon proxy, where the address of the implementation is + /// stored in the beacon. + pub trusted_address_slots: HashSet<(Address, U256)>, + /// Number of computational gas that validation step is allowed to use. + pub computational_gas_limit: u32, +} + +#[derive(Debug, Clone)] +pub enum ViolatedValidationRule { + TouchedUnallowedStorageSlots(Address, U256), + CalledContractWithNoCode(Address), + TouchedUnallowedContext, + TookTooManyComputationalGas(u32), +} + +impl fmt::Display for ViolatedValidationRule { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ViolatedValidationRule::TouchedUnallowedStorageSlots(contract, key) => write!( + f, + "Touched unallowed storage slots: address {contract:x}, key: {key:x}", + ), + ViolatedValidationRule::CalledContractWithNoCode(contract) => { + write!(f, "Called contract with no code: {contract:x}") + } + ViolatedValidationRule::TouchedUnallowedContext => { + write!(f, "Touched unallowed context") + } + ViolatedValidationRule::TookTooManyComputationalGas(gas_limit) => { + write!( + f, + "Took too many computational gas, allowed limit: {gas_limit}" + ) + } + } + } +} diff --git a/core/node/api_server/src/execution_sandbox/execute.rs b/core/node/api_server/src/execution_sandbox/execute.rs index 8b6203751c9..307188eceff 100644 --- a/core/node/api_server/src/execution_sandbox/execute.rs +++ b/core/node/api_server/src/execution_sandbox/execute.rs @@ -4,10 +4,10 @@ use async_trait::async_trait; use zksync_dal::{Connection, Core}; use zksync_multivm::interface::{ executor::OneshotExecutor, storage::ReadStorage, BytecodeCompressionError, OneshotEnv, - TransactionExecutionMetrics, TxExecutionArgs, VmExecutionResultAndLogs, + OneshotTracers, TransactionExecutionMetrics, TxExecutionArgs, VmExecutionResultAndLogs, }; use zksync_types::api::state_override::StateOverride; -use zksync_vm_executor::oneshot::{ApiTracer, MainOneshotExecutor, MockOneshotExecutor}; +use zksync_vm_executor::oneshot::{MainOneshotExecutor, MockOneshotExecutor}; use super::{apply, storage::StorageWithOverrides, vm_metrics, BlockArgs, TxSetupArgs, VmPermit}; @@ -46,7 +46,7 @@ impl TransactionExecutor { connection: Connection<'static, Core>, block_args: BlockArgs, state_override: Option, - tracers: Vec, + tracers: OneshotTracers<'_>, ) -> anyhow::Result { let total_factory_deps = execution_args.transaction.execute.factory_deps.len() as u16; let (env, storage) = @@ -80,14 +80,12 @@ impl OneshotExecutor for TransactionExecutor where S: ReadStorage + Send + 'static, { - type Tracers = Vec; - async fn inspect_transaction( &self, storage: S, env: OneshotEnv, args: TxExecutionArgs, - tracers: Self::Tracers, + tracers: OneshotTracers<'_>, ) -> anyhow::Result { match self { Self::Real(executor) => { @@ -95,7 +93,11 @@ where .inspect_transaction(storage, env, args, tracers) .await } - Self::Mock(executor) => executor.inspect_transaction(storage, env, args, ()).await, + Self::Mock(executor) => { + executor + .inspect_transaction(storage, env, args, tracers) + .await + } } } @@ -104,7 +106,7 @@ where storage: S, env: OneshotEnv, args: TxExecutionArgs, - tracers: Self::Tracers, + tracers: OneshotTracers<'_>, ) -> anyhow::Result<( Result<(), BytecodeCompressionError>, VmExecutionResultAndLogs, @@ -117,7 +119,7 @@ where } Self::Mock(executor) => { executor - .inspect_transaction_with_bytecode_compression(storage, env, args, ()) + .inspect_transaction_with_bytecode_compression(storage, env, args, tracers) .await } } diff --git a/core/node/api_server/src/execution_sandbox/validate.rs b/core/node/api_server/src/execution_sandbox/validate.rs index 3c3dca501ca..d5a48364822 100644 --- a/core/node/api_server/src/execution_sandbox/validate.rs +++ b/core/node/api_server/src/execution_sandbox/validate.rs @@ -4,14 +4,16 @@ use anyhow::Context as _; use tracing::Instrument; use zksync_dal::{Connection, Core, CoreDal}; use zksync_multivm::{ - interface::{executor::OneshotExecutor, ExecutionResult, TxExecutionArgs}, - tracers::{ValidationError as RawValidationError, ValidationTracerParams}, + interface::{ + executor::OneshotExecutor, tracer::ValidationTracerParams, ExecutionResult, OneshotTracers, + TxExecutionArgs, + }, + tracers::ValidationError as RawValidationError, }; use zksync_types::{ api::state_override::StateOverride, l2::L2Tx, Address, TRUSTED_ADDRESS_SLOTS, TRUSTED_TOKEN_SLOTS, }; -use zksync_vm_executor::oneshot::ApiTracer; use super::{ apply, @@ -56,20 +58,28 @@ impl TransactionExecutor { apply::prepare_env_and_storage(connection, setup_args, &block_args).await?; let storage = StorageWithOverrides::new(storage, &StateOverride::default()); + let mut validation_result = Ok(()); let execution_args = TxExecutionArgs::for_validation(tx); - let (tracer, validation_result) = ApiTracer::validation(params); let stage_latency = SANDBOX_METRICS.sandbox[&SandboxStage::Validation].start(); let result = self - .inspect_transaction(storage, env, execution_args, vec![tracer]) + .inspect_transaction( + storage, + env, + execution_args, + OneshotTracers::Validation { + params: ¶ms, + result: &mut validation_result, + }, + ) .instrument(tracing::debug_span!("validation")) .await?; drop(vm_permit); stage_latency.observe(); - let validation_result = match (result.result, validation_result.get()) { - (_, Some(rule)) => Err(RawValidationError::ViolatedRule(rule.clone())), + let validation_result = match (result.result, validation_result) { + (_, Err(rule)) => Err(RawValidationError::ViolatedRule(rule)), (ExecutionResult::Halt { reason }, _) => Err(RawValidationError::FailedTx(reason)), - (_, None) => Ok(()), + (_, Ok(())) => Ok(()), }; total_latency.observe(); validation_result.map_err(ValidationError::Vm) diff --git a/core/node/api_server/src/tx_sender/mod.rs b/core/node/api_server/src/tx_sender/mod.rs index c01705669f3..87cac48431c 100644 --- a/core/node/api_server/src/tx_sender/mod.rs +++ b/core/node/api_server/src/tx_sender/mod.rs @@ -11,7 +11,8 @@ use zksync_dal::{ }; use zksync_multivm::{ interface::{ - TransactionExecutionMetrics, TxExecutionArgs, TxExecutionMode, VmExecutionResultAndLogs, + OneshotTracers, TransactionExecutionMetrics, TxExecutionArgs, TxExecutionMode, + VmExecutionResultAndLogs, }, utils::{ adjust_pubdata_price_for_tx, derive_base_fee_and_gas_per_pubdata, derive_overhead, @@ -390,7 +391,7 @@ impl TxSender { connection, block_args, None, - vec![], + OneshotTracers::None, ) .await?; tracing::info!( @@ -727,7 +728,7 @@ impl TxSender { connection, block_args, state_override, - vec![], + OneshotTracers::None, ) .await?; Ok((execution_output.vm, execution_output.metrics)) @@ -1027,7 +1028,7 @@ impl TxSender { connection, block_args, state_override, - vec![], + OneshotTracers::None, ) .await?; result.vm.into_api_call_result() diff --git a/core/node/api_server/src/web3/namespaces/debug.rs b/core/node/api_server/src/web3/namespaces/debug.rs index 053fb338324..dd8593acffc 100644 --- a/core/node/api_server/src/web3/namespaces/debug.rs +++ b/core/node/api_server/src/web3/namespaces/debug.rs @@ -1,10 +1,9 @@ -use std::sync::Arc; - use anyhow::Context as _; -use once_cell::sync::OnceCell; use zksync_dal::{CoreDal, DalError}; use zksync_multivm::{ - interface::{Call, CallType, ExecutionResult, TxExecutionArgs, TxExecutionMode}, + interface::{ + Call, CallType, ExecutionResult, OneshotTracers, TxExecutionArgs, TxExecutionMode, + }, vm_latest::constants::BATCH_COMPUTATIONAL_GAS_LIMIT, }; use zksync_system_constants::MAX_ENCODED_TX_SIZE; @@ -16,7 +15,6 @@ use zksync_types::{ transaction_request::CallRequest, web3, AccountTreeId, H256, U256, }; -use zksync_vm_executor::oneshot::ApiTracer; use zksync_web3_decl::error::Web3Error; use crate::{ @@ -191,11 +189,11 @@ impl DebugNamespace { let vm_permit = vm_permit.context("cannot acquire VM permit")?; // We don't need properly trace if we only need top call - let call_tracer_result = Arc::new(OnceCell::default()); + let mut calls = vec![]; let custom_tracers = if only_top_call { - vec![] + OneshotTracers::None } else { - vec![ApiTracer::CallTracer(call_tracer_result.clone())] + OneshotTracers::Calls(&mut calls) }; let connection = self.state.acquire_connection().await?; @@ -224,11 +222,6 @@ impl DebugNamespace { } }; - // We had only one copy of Arc this arc is already dropped it's safe to unwrap - let trace = Arc::try_unwrap(call_tracer_result) - .unwrap() - .take() - .unwrap_or_default(); let call = Call::new_high_level( tx.common_data.fee.gas_limit.as_u64(), result.statistics.gas_used, @@ -236,7 +229,7 @@ impl DebugNamespace { tx.execute.calldata, output, revert_reason, - trace, + calls, ); Ok(Self::map_call(call, false)) } From db94a9a9ded44347f133393a518c9d45c8d3070d Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 3 Sep 2024 17:46:42 +0300 Subject: [PATCH 03/11] Rework `OneshotExecutor` methods --- core/lib/multivm/src/tracers/mod.rs | 7 +- core/lib/multivm/src/tracers/validator/mod.rs | 9 +- .../multivm/src/tracers/validator/types.rs | 23 ----- core/lib/vm_executor/src/oneshot/mock.rs | 31 ++++--- core/lib/vm_executor/src/oneshot/mod.rs | 93 ++++++++++++------- core/lib/vm_executor/src/oneshot/tracers.rs | 72 -------------- core/lib/vm_interface/src/executor.rs | 18 ++-- core/lib/vm_interface/src/lib.rs | 9 +- core/lib/vm_interface/src/types/inputs/mod.rs | 17 +--- .../src/types/outputs/execution_result.rs | 19 +++- .../lib/vm_interface/src/types/outputs/mod.rs | 6 +- core/lib/vm_interface/src/types/tracer.rs | 21 ++++- .../src/execution_sandbox/execute.rs | 58 ++++++++---- .../src/execution_sandbox/validate.rs | 34 ++----- core/node/api_server/src/tx_sender/mod.rs | 8 +- .../api_server/src/web3/namespaces/debug.rs | 20 ++-- 16 files changed, 201 insertions(+), 244 deletions(-) delete mode 100644 core/lib/vm_executor/src/oneshot/tracers.rs diff --git a/core/lib/multivm/src/tracers/mod.rs b/core/lib/multivm/src/tracers/mod.rs index 8dacfaad4ae..35224d993a1 100644 --- a/core/lib/multivm/src/tracers/mod.rs +++ b/core/lib/multivm/src/tracers/mod.rs @@ -1,9 +1,6 @@ pub use self::{ - call_tracer::CallTracer, - multivm_dispatcher::TracerDispatcher, - prestate_tracer::PrestateTracer, - storage_invocation::StorageInvocations, - validator::{ValidationError, ValidationTracer}, + call_tracer::CallTracer, multivm_dispatcher::TracerDispatcher, prestate_tracer::PrestateTracer, + storage_invocation::StorageInvocations, validator::ValidationTracer, }; mod call_tracer; diff --git a/core/lib/multivm/src/tracers/validator/mod.rs b/core/lib/multivm/src/tracers/validator/mod.rs index 06129db40ae..a1573f24c66 100644 --- a/core/lib/multivm/src/tracers/validator/mod.rs +++ b/core/lib/multivm/src/tracers/validator/mod.rs @@ -10,13 +10,12 @@ use zksync_types::{ }; use zksync_utils::{be_bytes_to_safe_address, u256_to_account_address, u256_to_h256}; -pub use self::types::ValidationError; use self::types::{NewTrustedValidationItems, ValidationTracerMode}; use crate::{ glue::tracers::IntoOldVmTracer, interface::{ storage::{StoragePtr, WriteStorage}, - tracer::{ValidationTracerParams, ViolatedValidationRule}, + tracer::{ValidationParams, ViolatedValidationRule}, }, }; @@ -53,7 +52,7 @@ type ValidationRoundResult = Result ValidationTracer { pub fn new( - params: ValidationTracerParams, + params: ValidationParams, vm_version: VmVersion, ) -> (Self, Arc>) { let result = Arc::new(OnceCell::new()); @@ -182,8 +181,8 @@ impl ValidationTracer { } } - pub fn params(&self) -> ValidationTracerParams { - ValidationTracerParams { + pub fn params(&self) -> ValidationParams { + ValidationParams { user_address: self.user_address, paymaster_address: self.paymaster_address, trusted_slots: self.trusted_slots.clone(), diff --git a/core/lib/multivm/src/tracers/validator/types.rs b/core/lib/multivm/src/tracers/validator/types.rs index a57dddef33e..b9d44227992 100644 --- a/core/lib/multivm/src/tracers/validator/types.rs +++ b/core/lib/multivm/src/tracers/validator/types.rs @@ -1,9 +1,5 @@ -use std::fmt::Display; - use zksync_types::{Address, H256}; -use crate::interface::{tracer::ViolatedValidationRule, Halt}; - #[derive(Debug, Clone, Eq, PartialEq, Copy)] #[allow(clippy::enum_variant_names)] pub(super) enum ValidationTracerMode { @@ -20,22 +16,3 @@ pub(super) struct NewTrustedValidationItems { pub(super) new_allowed_slots: Vec, pub(super) new_trusted_addresses: Vec
, } - -#[derive(Debug, Clone)] -pub enum ValidationError { - FailedTx(Halt), - ViolatedRule(ViolatedValidationRule), -} - -impl Display for ValidationError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::FailedTx(revert_reason) => { - write!(f, "Validation revert: {}", revert_reason) - } - Self::ViolatedRule(rule) => { - write!(f, "Violated validation rules: {}", rule) - } - } - } -} diff --git a/core/lib/vm_executor/src/oneshot/mock.rs b/core/lib/vm_executor/src/oneshot/mock.rs index 510a43a2ae9..642b2598767 100644 --- a/core/lib/vm_executor/src/oneshot/mock.rs +++ b/core/lib/vm_executor/src/oneshot/mock.rs @@ -2,8 +2,11 @@ use std::fmt; use async_trait::async_trait; use zksync_multivm::interface::{ - executor::OneshotExecutor, storage::ReadStorage, BytecodeCompressionError, ExecutionResult, - OneshotEnv, OneshotTracers, TxExecutionArgs, TxExecutionMode, VmExecutionResultAndLogs, + executor::OneshotExecutor, + storage::ReadStorage, + tracer::{ValidationError, ValidationParams}, + ExecutionResult, OneshotEnv, OneshotTracingParams, OneshotTransactionExecutionResult, + TxExecutionArgs, TxExecutionMode, VmExecutionResultAndLogs, }; use zksync_types::Transaction; @@ -88,14 +91,17 @@ impl OneshotExecutor for MockOneshotExecutor where S: ReadStorage + Send + 'static, { - async fn inspect_transaction( + async fn validate_transaction( &self, _storage: S, env: OneshotEnv, args: TxExecutionArgs, - _tracers: OneshotTracers<'_>, - ) -> anyhow::Result { - Ok(self.mock_inspect(env, args)) + _validation_params: ValidationParams, + ) -> anyhow::Result> { + Ok(match self.mock_inspect(env, args).result { + ExecutionResult::Halt { reason } => Err(ValidationError::FailedTx(reason)), + ExecutionResult::Success { .. } | ExecutionResult::Revert { .. } => Ok(()), + }) } async fn inspect_transaction_with_bytecode_compression( @@ -103,11 +109,12 @@ where _storage: S, env: OneshotEnv, args: TxExecutionArgs, - _tracers: OneshotTracers<'_>, - ) -> anyhow::Result<( - Result<(), BytecodeCompressionError>, - VmExecutionResultAndLogs, - )> { - Ok((Ok(()), self.mock_inspect(env, args))) + _params: OneshotTracingParams, + ) -> anyhow::Result { + Ok(OneshotTransactionExecutionResult { + tx_result: Box::new(self.mock_inspect(env, args)), + compression_result: Ok(()), + call_traces: vec![], + }) } } diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs index d075c6043e0..b6e747f0e7b 100644 --- a/core/lib/vm_executor/src/oneshot/mod.rs +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -1,17 +1,22 @@ //! Oneshot VM executor. -use std::time::{Duration, Instant}; +use std::{ + sync::Arc, + time::{Duration, Instant}, +}; use anyhow::Context; use async_trait::async_trait; +use once_cell::sync::OnceCell; use zksync_multivm::{ interface::{ executor::OneshotExecutor, storage::{ReadStorage, StoragePtr, StorageView, WriteStorage}, - BytecodeCompressionError, OneshotEnv, OneshotTracers, StoredL2BlockEnv, TxExecutionArgs, - TxExecutionMode, VmExecutionMode, VmExecutionResultAndLogs, VmInterface, + tracer::{ValidationError, ValidationParams}, + ExecutionResult, OneshotEnv, OneshotTracingParams, OneshotTransactionExecutionResult, + StoredL2BlockEnv, TxExecutionArgs, TxExecutionMode, VmExecutionMode, VmInterface, }, - tracers::StorageInvocations, + tracers::{CallTracer, StorageInvocations, ValidationTracer}, utils::adjust_pubdata_price_for_tx, vm_latest::HistoryDisabled, zk_evm_latest::ethereum_types::U256, @@ -27,11 +32,9 @@ use zksync_types::{ use zksync_utils::{h256_to_u256, u256_to_h256}; pub use self::mock::MockOneshotExecutor; -use self::tracers::TracersAdapter; mod metrics; mod mock; -mod tracers; /// Main [`OneshotExecutor`] implementation used by the API server. #[derive(Debug, Default)] @@ -54,15 +57,13 @@ impl OneshotExecutor for MainOneshotExecutor where S: ReadStorage + Send + 'static, { - async fn inspect_transaction( + async fn validate_transaction( &self, storage: S, env: OneshotEnv, args: TxExecutionArgs, - tracers: OneshotTracers<'_>, - ) -> anyhow::Result { - let tracers = TracersAdapter::new(tracers); - let mut vm_tracers = tracers.to_boxed(env.system.version); + validation_params: ValidationParams, + ) -> anyhow::Result> { let missed_storage_invocation_limit = match env.system.execution_mode { // storage accesses are not limited for tx validation TxExecutionMode::VerifyExecute => usize::MAX, @@ -70,20 +71,35 @@ where self.missed_storage_invocation_limit } }; - vm_tracers - .push(StorageInvocations::new(missed_storage_invocation_limit).into_tracer_pointer()); - let result = tokio::task::spawn_blocking(move || { + tokio::task::spawn_blocking(move || { + let (validation_tracer, mut validation_result) = + ValidationTracer::::new( + validation_params, + env.system.version.into(), + ); + let tracers = vec![ + validation_tracer.into_tracer_pointer(), + StorageInvocations::new(missed_storage_invocation_limit).into_tracer_pointer(), + ]; + let executor = VmSandbox::new(storage, env, args); - executor.apply(|vm, transaction| { + let exec_result = executor.apply(|vm, transaction| { vm.push_transaction(transaction); - vm.inspect(vm_tracers.into(), VmExecutionMode::OneTx) - }) + vm.inspect(tracers.into(), VmExecutionMode::OneTx) + }); + let validation_result = Arc::make_mut(&mut validation_result) + .take() + .map_or(Ok(()), Err); + + match (exec_result.result, validation_result) { + (_, Err(violated_rule)) => Err(ValidationError::ViolatedRule(violated_rule)), + (ExecutionResult::Halt { reason }, _) => Err(ValidationError::FailedTx(reason)), + _ => Ok(()), + } }) .await - .context("VM execution panicked")?; - - Ok(result) + .context("VM execution panicked") } async fn inspect_transaction_with_bytecode_compression( @@ -91,13 +107,8 @@ where storage: S, env: OneshotEnv, args: TxExecutionArgs, - tracers: OneshotTracers<'_>, - ) -> anyhow::Result<( - Result<(), BytecodeCompressionError>, - VmExecutionResultAndLogs, - )> { - let tracers = TracersAdapter::new(tracers); - let mut vm_tracers = tracers.to_boxed(env.system.version); + params: OneshotTracingParams, + ) -> anyhow::Result { let missed_storage_invocation_limit = match env.system.execution_mode { // storage accesses are not limited for tx validation TxExecutionMode::VerifyExecute => usize::MAX, @@ -105,20 +116,34 @@ where self.missed_storage_invocation_limit } }; - vm_tracers - .push(StorageInvocations::new(missed_storage_invocation_limit).into_tracer_pointer()); 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(storage, env, args); - executor.apply(|vm, transaction| { - let (bytecodes_result, exec_result) = vm + let mut result = executor.apply(|vm, transaction| { + let (compression_result, tx_result) = vm .inspect_transaction_with_bytecode_compression( - vm_tracers.into(), + tracers.into(), transaction, true, ); - (bytecodes_result.map(drop), exec_result) - }) + 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 }) .await .context("VM execution panicked") diff --git a/core/lib/vm_executor/src/oneshot/tracers.rs b/core/lib/vm_executor/src/oneshot/tracers.rs deleted file mode 100644 index fae9df45648..00000000000 --- a/core/lib/vm_executor/src/oneshot/tracers.rs +++ /dev/null @@ -1,72 +0,0 @@ -use std::{sync::Arc, thread}; - -use once_cell::sync::OnceCell; -use zksync_multivm::{ - interface::{storage::WriteStorage, tracer::ViolatedValidationRule, Call, OneshotTracers}, - tracers::{CallTracer, ValidationTracer}, - vm_latest::HistoryDisabled, - MultiVMTracer, MultiVmTracerPointer, -}; -use zksync_types::ProtocolVersionId; - -/// Custom tracers supported by the API sandbox. -#[derive(Debug)] -pub(super) struct TracersAdapter<'a> { - inner: OneshotTracers<'a>, - calls_cell: Arc>>, - validation_cell: Arc>, -} - -impl<'a> TracersAdapter<'a> { - pub fn new(inner: OneshotTracers<'a>) -> Self { - Self { - inner, - calls_cell: Arc::default(), - validation_cell: Arc::default(), - } - } - - pub fn to_boxed( - &self, - protocol_version: ProtocolVersionId, - ) -> Vec> - where - S: WriteStorage, - { - match self.inner { - OneshotTracers::None => vec![], - OneshotTracers::Calls(_) => { - vec![CallTracer::new(self.calls_cell.clone()).into_tracer_pointer()] - } - OneshotTracers::Validation { params, .. } => { - let (mut tracer, _) = ValidationTracer::::new( - params.clone(), - protocol_version.into(), - ); - tracer.result = self.validation_cell.clone(); - vec![tracer.into_tracer_pointer()] - } - } - } -} - -impl Drop for TracersAdapter<'_> { - fn drop(&mut self) { - if thread::panicking() { - return; - } - - match &mut self.inner { - OneshotTracers::None => { /* do nothing */ } - OneshotTracers::Calls(calls) => { - **calls = self.calls_cell.get().cloned().unwrap_or_default(); - } - OneshotTracers::Validation { result, .. } => { - **result = match self.validation_cell.get() { - Some(rule) => Err(rule.clone()), - None => Ok(()), - }; - } - } - } -} diff --git a/core/lib/vm_interface/src/executor.rs b/core/lib/vm_interface/src/executor.rs index d07414791c2..6a5a5ea5158 100644 --- a/core/lib/vm_interface/src/executor.rs +++ b/core/lib/vm_interface/src/executor.rs @@ -7,8 +7,9 @@ use zksync_types::Transaction; use crate::{ storage::{ReadStorage, StorageView}, - BatchTransactionExecutionResult, BytecodeCompressionError, FinishedL1Batch, L1BatchEnv, - L2BlockEnv, OneshotEnv, OneshotTracers, SystemEnv, TxExecutionArgs, VmExecutionResultAndLogs, + tracer::{ValidationError, ValidationParams}, + BatchTransactionExecutionResult, FinishedL1Batch, L1BatchEnv, L2BlockEnv, OneshotEnv, + OneshotTracingParams, OneshotTransactionExecutionResult, SystemEnv, TxExecutionArgs, }; /// Factory of [`BatchExecutor`]s. @@ -47,22 +48,19 @@ pub trait BatchExecutor: 'static + Send + fmt::Debug { /// VM executor capable of executing isolated transactions / calls (as opposed to [batch execution](BatchExecutor)). #[async_trait] pub trait OneshotExecutor { - async fn inspect_transaction( + async fn validate_transaction( &self, storage: S, env: OneshotEnv, args: TxExecutionArgs, - tracers: OneshotTracers<'_>, - ) -> anyhow::Result; + validation_params: ValidationParams, + ) -> anyhow::Result>; async fn inspect_transaction_with_bytecode_compression( &self, storage: S, env: OneshotEnv, args: TxExecutionArgs, - tracers: OneshotTracers<'_>, - ) -> anyhow::Result<( - Result<(), BytecodeCompressionError>, - VmExecutionResultAndLogs, - )>; + tracing: OneshotTracingParams, + ) -> anyhow::Result; } diff --git a/core/lib/vm_interface/src/lib.rs b/core/lib/vm_interface/src/lib.rs index 24cdb0936bb..2b30f82e0ce 100644 --- a/core/lib/vm_interface/src/lib.rs +++ b/core/lib/vm_interface/src/lib.rs @@ -24,15 +24,16 @@ pub use crate::{ VmRevertReason, VmRevertReasonParsingError, }, inputs::{ - L1BatchEnv, L2BlockEnv, OneshotEnv, OneshotTracers, StoredL2BlockEnv, SystemEnv, + L1BatchEnv, L2BlockEnv, OneshotEnv, OneshotTracingParams, StoredL2BlockEnv, SystemEnv, TxExecutionArgs, TxExecutionMode, VmExecutionMode, }, outputs::{ BatchTransactionExecutionResult, BootloaderMemory, Call, CallType, CircuitStatistic, CompressedBytecodeInfo, CurrentExecutionState, DeduplicatedWritesMetrics, - ExecutionResult, FinishedL1Batch, L2Block, Refunds, TransactionExecutionMetrics, - TransactionExecutionResult, TxExecutionStatus, VmEvent, VmExecutionLogs, - VmExecutionMetrics, VmExecutionResultAndLogs, VmExecutionStatistics, VmMemoryMetrics, + ExecutionResult, FinishedL1Batch, L2Block, OneshotTransactionExecutionResult, Refunds, + TransactionExecutionMetrics, TransactionExecutionResult, TxExecutionStatus, VmEvent, + VmExecutionLogs, VmExecutionMetrics, VmExecutionResultAndLogs, VmExecutionStatistics, + VmMemoryMetrics, }, tracer, }, diff --git a/core/lib/vm_interface/src/types/inputs/mod.rs b/core/lib/vm_interface/src/types/inputs/mod.rs index 1f738a9d30c..24f58ae72f1 100644 --- a/core/lib/vm_interface/src/types/inputs/mod.rs +++ b/core/lib/vm_interface/src/types/inputs/mod.rs @@ -8,10 +8,6 @@ pub use self::{ l2_block::{L2BlockEnv, StoredL2BlockEnv}, system_env::{SystemEnv, TxExecutionMode}, }; -use crate::{ - tracer::{ValidationTracerParams, ViolatedValidationRule}, - Call, -}; mod execution_mode; mod l1_batch_env; @@ -91,12 +87,9 @@ impl TxExecutionArgs { } } -#[derive(Debug)] -pub enum OneshotTracers<'a> { - None, - Calls(&'a mut Vec), - Validation { - params: &'a ValidationTracerParams, - result: &'a mut Result<(), ViolatedValidationRule>, - }, +/// Inputs and outputs for all tracers supported for oneshot transaction / call execution. +#[derive(Debug, Default)] +pub struct OneshotTracingParams { + /// Whether to trace contract calls. + pub trace_calls: bool, } diff --git a/core/lib/vm_interface/src/types/outputs/execution_result.rs b/core/lib/vm_interface/src/types/outputs/execution_result.rs index d74d74652e2..6f9c02f0b58 100644 --- a/core/lib/vm_interface/src/types/outputs/execution_result.rs +++ b/core/lib/vm_interface/src/types/outputs/execution_result.rs @@ -11,7 +11,8 @@ use zksync_types::{ }; use crate::{ - CompressedBytecodeInfo, Halt, VmExecutionMetrics, VmExecutionStatistics, VmRevertReason, + BytecodeCompressionError, CompressedBytecodeInfo, Halt, VmExecutionMetrics, + VmExecutionStatistics, VmRevertReason, }; const L1_MESSAGE_EVENT_SIGNATURE: H256 = H256([ @@ -297,11 +298,14 @@ impl Call { } } -/// Mid-level transaction execution output returned by a batch executor. +/// Mid-level transaction execution output returned by a [batch executor](crate::executor::BatchExecutor). #[derive(Debug, Clone)] pub struct BatchTransactionExecutionResult { + /// VM result. pub tx_result: Box, + /// Compressed bytecodes used by the transaction. pub compressed_bytecodes: Vec, + /// Call traces (if requested; otherwise, empty). pub call_traces: Vec, } @@ -311,6 +315,17 @@ impl BatchTransactionExecutionResult { } } +/// Mid-level transaction execution output returned by a [oneshot executor](crate::executor::OneshotExecutor). +#[derive(Debug)] +pub struct OneshotTransactionExecutionResult { + /// VM result. + pub tx_result: Box, + /// Result of compressing bytecodes used by the transaction. + pub compression_result: Result<(), BytecodeCompressionError>, + /// Call traces (if requested; otherwise, empty). + pub call_traces: Vec, +} + /// High-level transaction execution result used by the API server sandbox etc. #[derive(Debug, Clone, PartialEq)] pub struct TransactionExecutionResult { diff --git a/core/lib/vm_interface/src/types/outputs/mod.rs b/core/lib/vm_interface/src/types/outputs/mod.rs index abefa59bbe7..1fa1cd5d168 100644 --- a/core/lib/vm_interface/src/types/outputs/mod.rs +++ b/core/lib/vm_interface/src/types/outputs/mod.rs @@ -1,9 +1,9 @@ pub use self::{ bytecode::CompressedBytecodeInfo, execution_result::{ - BatchTransactionExecutionResult, Call, CallType, ExecutionResult, Refunds, - TransactionExecutionResult, TxExecutionStatus, VmEvent, VmExecutionLogs, - VmExecutionResultAndLogs, + BatchTransactionExecutionResult, Call, CallType, ExecutionResult, + OneshotTransactionExecutionResult, Refunds, TransactionExecutionResult, TxExecutionStatus, + VmEvent, VmExecutionLogs, VmExecutionResultAndLogs, }, execution_state::{BootloaderMemory, CurrentExecutionState}, finished_l1batch::FinishedL1Batch, diff --git a/core/lib/vm_interface/src/types/tracer.rs b/core/lib/vm_interface/src/types/tracer.rs index dc83287cd51..e743f8e22af 100644 --- a/core/lib/vm_interface/src/types/tracer.rs +++ b/core/lib/vm_interface/src/types/tracer.rs @@ -43,7 +43,7 @@ pub enum VmExecutionStopReason { } #[derive(Debug, Clone)] -pub struct ValidationTracerParams { +pub struct ValidationParams { pub user_address: Address, pub paymaster_address: Address, /// Slots that are trusted (i.e. the user can access them). @@ -88,3 +88,22 @@ impl fmt::Display for ViolatedValidationRule { } } } + +#[derive(Debug)] +pub enum ValidationError { + FailedTx(Halt), + ViolatedRule(ViolatedValidationRule), +} + +impl fmt::Display for ValidationError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::FailedTx(revert_reason) => { + write!(f, "Validation revert: {}", revert_reason) + } + Self::ViolatedRule(rule) => { + write!(f, "Violated validation rules: {}", rule) + } + } + } +} diff --git a/core/node/api_server/src/execution_sandbox/execute.rs b/core/node/api_server/src/execution_sandbox/execute.rs index 307188eceff..aa6b2c4171a 100644 --- a/core/node/api_server/src/execution_sandbox/execute.rs +++ b/core/node/api_server/src/execution_sandbox/execute.rs @@ -3,8 +3,11 @@ use async_trait::async_trait; use zksync_dal::{Connection, Core}; use zksync_multivm::interface::{ - executor::OneshotExecutor, storage::ReadStorage, BytecodeCompressionError, OneshotEnv, - OneshotTracers, TransactionExecutionMetrics, TxExecutionArgs, VmExecutionResultAndLogs, + executor::OneshotExecutor, + storage::ReadStorage, + tracer::{ValidationError, ValidationParams}, + Call, OneshotEnv, OneshotTracingParams, OneshotTransactionExecutionResult, + TransactionExecutionMetrics, TxExecutionArgs, VmExecutionResultAndLogs, }; use zksync_types::api::state_override::StateOverride; use zksync_vm_executor::oneshot::{MainOneshotExecutor, MockOneshotExecutor}; @@ -15,6 +18,8 @@ use super::{apply, storage::StorageWithOverrides, vm_metrics, BlockArgs, TxSetup pub(crate) struct TransactionExecutionOutput { /// Output of the VM. pub vm: VmExecutionResultAndLogs, + /// Traced calls if requested. + pub call_traces: Vec, /// Execution metrics. pub metrics: TransactionExecutionMetrics, /// Were published bytecodes OK? @@ -46,7 +51,7 @@ impl TransactionExecutor { connection: Connection<'static, Core>, block_args: BlockArgs, state_override: Option, - tracers: OneshotTracers<'_>, + tracing_params: OneshotTracingParams, ) -> anyhow::Result { let total_factory_deps = execution_args.transaction.execute.factory_deps.len() as u16; let (env, storage) = @@ -54,17 +59,23 @@ impl TransactionExecutor { let state_override = state_override.unwrap_or_default(); let storage = StorageWithOverrides::new(storage, &state_override); - let (published_bytecodes, execution_result) = self - .inspect_transaction_with_bytecode_compression(storage, env, execution_args, tracers) + let result = self + .inspect_transaction_with_bytecode_compression( + storage, + env, + execution_args, + tracing_params, + ) .await?; drop(vm_permit); let metrics = - vm_metrics::collect_tx_execution_metrics(total_factory_deps, &execution_result); + vm_metrics::collect_tx_execution_metrics(total_factory_deps, &result.tx_result); Ok(TransactionExecutionOutput { - vm: execution_result, + vm: *result.tx_result, + call_traces: result.call_traces, metrics, - are_published_bytecodes_ok: published_bytecodes.is_ok(), + are_published_bytecodes_ok: result.compression_result.is_ok(), }) } } @@ -80,22 +91,22 @@ impl OneshotExecutor for TransactionExecutor where S: ReadStorage + Send + 'static, { - async fn inspect_transaction( + async fn validate_transaction( &self, storage: S, env: OneshotEnv, args: TxExecutionArgs, - tracers: OneshotTracers<'_>, - ) -> anyhow::Result { + validation_params: ValidationParams, + ) -> anyhow::Result> { match self { Self::Real(executor) => { executor - .inspect_transaction(storage, env, args, tracers) + .validate_transaction(storage, env, args, validation_params) .await } Self::Mock(executor) => { executor - .inspect_transaction(storage, env, args, tracers) + .validate_transaction(storage, env, args, validation_params) .await } } @@ -106,20 +117,27 @@ where storage: S, env: OneshotEnv, args: TxExecutionArgs, - tracers: OneshotTracers<'_>, - ) -> anyhow::Result<( - Result<(), BytecodeCompressionError>, - VmExecutionResultAndLogs, - )> { + tracing_params: OneshotTracingParams, + ) -> anyhow::Result { match self { Self::Real(executor) => { executor - .inspect_transaction_with_bytecode_compression(storage, env, args, tracers) + .inspect_transaction_with_bytecode_compression( + storage, + env, + args, + tracing_params, + ) .await } Self::Mock(executor) => { executor - .inspect_transaction_with_bytecode_compression(storage, env, args, tracers) + .inspect_transaction_with_bytecode_compression( + storage, + env, + args, + tracing_params, + ) .await } } diff --git a/core/node/api_server/src/execution_sandbox/validate.rs b/core/node/api_server/src/execution_sandbox/validate.rs index d5a48364822..24ff1c61e44 100644 --- a/core/node/api_server/src/execution_sandbox/validate.rs +++ b/core/node/api_server/src/execution_sandbox/validate.rs @@ -3,12 +3,10 @@ use std::collections::HashSet; use anyhow::Context as _; use tracing::Instrument; use zksync_dal::{Connection, Core, CoreDal}; -use zksync_multivm::{ - interface::{ - executor::OneshotExecutor, tracer::ValidationTracerParams, ExecutionResult, OneshotTracers, - TxExecutionArgs, - }, - tracers::ValidationError as RawValidationError, +use zksync_multivm::interface::{ + executor::OneshotExecutor, + tracer::{ValidationError as RawValidationError, ValidationParams}, + TxExecutionArgs, }; use zksync_types::{ api::state_override::StateOverride, l2::L2Tx, Address, TRUSTED_ADDRESS_SLOTS, @@ -45,7 +43,7 @@ impl TransactionExecutor { computational_gas_limit: u32, ) -> Result<(), ValidationError> { let total_latency = SANDBOX_METRICS.sandbox[&SandboxStage::ValidateInSandbox].start(); - let params = get_validation_params( + let validation_params = get_validation_params( &mut connection, &tx, computational_gas_limit, @@ -58,29 +56,15 @@ impl TransactionExecutor { apply::prepare_env_and_storage(connection, setup_args, &block_args).await?; let storage = StorageWithOverrides::new(storage, &StateOverride::default()); - let mut validation_result = Ok(()); let execution_args = TxExecutionArgs::for_validation(tx); let stage_latency = SANDBOX_METRICS.sandbox[&SandboxStage::Validation].start(); - let result = self - .inspect_transaction( - storage, - env, - execution_args, - OneshotTracers::Validation { - params: ¶ms, - result: &mut validation_result, - }, - ) + let validation_result = self + .validate_transaction(storage, env, execution_args, validation_params) .instrument(tracing::debug_span!("validation")) .await?; drop(vm_permit); stage_latency.observe(); - let validation_result = match (result.result, validation_result) { - (_, Err(rule)) => Err(RawValidationError::ViolatedRule(rule)), - (ExecutionResult::Halt { reason }, _) => Err(RawValidationError::FailedTx(reason)), - (_, Ok(())) => Ok(()), - }; total_latency.observe(); validation_result.map_err(ValidationError::Vm) } @@ -94,7 +78,7 @@ async fn get_validation_params( tx: &L2Tx, computational_gas_limit: u32, whitelisted_tokens_for_aa: &[Address], -) -> anyhow::Result { +) -> anyhow::Result { let method_latency = EXECUTION_METRICS.get_validation_params.start(); let user_address = tx.common_data.initiator_address; let paymaster_address = tx.common_data.paymaster_params.paymaster; @@ -133,7 +117,7 @@ async fn get_validation_params( span.exit(); method_latency.observe(); - Ok(ValidationTracerParams { + Ok(ValidationParams { user_address, paymaster_address, trusted_slots, diff --git a/core/node/api_server/src/tx_sender/mod.rs b/core/node/api_server/src/tx_sender/mod.rs index 87cac48431c..e69b4100fba 100644 --- a/core/node/api_server/src/tx_sender/mod.rs +++ b/core/node/api_server/src/tx_sender/mod.rs @@ -11,7 +11,7 @@ use zksync_dal::{ }; use zksync_multivm::{ interface::{ - OneshotTracers, TransactionExecutionMetrics, TxExecutionArgs, TxExecutionMode, + OneshotTracingParams, TransactionExecutionMetrics, TxExecutionArgs, TxExecutionMode, VmExecutionResultAndLogs, }, utils::{ @@ -391,7 +391,7 @@ impl TxSender { connection, block_args, None, - OneshotTracers::None, + OneshotTracingParams::default(), ) .await?; tracing::info!( @@ -728,7 +728,7 @@ impl TxSender { connection, block_args, state_override, - OneshotTracers::None, + OneshotTracingParams::default(), ) .await?; Ok((execution_output.vm, execution_output.metrics)) @@ -1028,7 +1028,7 @@ impl TxSender { connection, block_args, state_override, - OneshotTracers::None, + OneshotTracingParams::default(), ) .await?; result.vm.into_api_call_result() diff --git a/core/node/api_server/src/web3/namespaces/debug.rs b/core/node/api_server/src/web3/namespaces/debug.rs index dd8593acffc..ad00f6a878b 100644 --- a/core/node/api_server/src/web3/namespaces/debug.rs +++ b/core/node/api_server/src/web3/namespaces/debug.rs @@ -2,7 +2,7 @@ use anyhow::Context as _; use zksync_dal::{CoreDal, DalError}; use zksync_multivm::{ interface::{ - Call, CallType, ExecutionResult, OneshotTracers, TxExecutionArgs, TxExecutionMode, + Call, CallType, ExecutionResult, OneshotTracingParams, TxExecutionArgs, TxExecutionMode, }, vm_latest::constants::BATCH_COMPUTATIONAL_GAS_LIMIT, }; @@ -189,11 +189,8 @@ impl DebugNamespace { let vm_permit = vm_permit.context("cannot acquire VM permit")?; // We don't need properly trace if we only need top call - let mut calls = vec![]; - let custom_tracers = if only_top_call { - OneshotTracers::None - } else { - OneshotTracers::Calls(&mut calls) + let tracing_params = OneshotTracingParams { + trace_calls: !only_top_call, }; let connection = self.state.acquire_connection().await?; @@ -206,12 +203,11 @@ impl DebugNamespace { connection, block_args, None, - custom_tracers, + tracing_params, ) - .await? - .vm; + .await?; - let (output, revert_reason) = match result.result { + let (output, revert_reason) = match result.vm.result { ExecutionResult::Success { output, .. } => (output, None), ExecutionResult::Revert { output } => (vec![], Some(output.to_string())), ExecutionResult::Halt { reason } => { @@ -224,12 +220,12 @@ impl DebugNamespace { let call = Call::new_high_level( tx.common_data.fee.gas_limit.as_u64(), - result.statistics.gas_used, + result.vm.statistics.gas_used, tx.execute.value, tx.execute.calldata, output, revert_reason, - calls, + result.call_traces, ); Ok(Self::map_call(call, false)) } From c06d947683680f2817b4b7058cf818f5ecf224e6 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 4 Sep 2024 14:22:13 +0300 Subject: [PATCH 04/11] Split off tx validation into separate trait --- .../src/tracers/validator/vm_1_4_1/mod.rs | 4 +- .../src/tracers/validator/vm_1_4_2/mod.rs | 4 +- .../validator/vm_boojum_integration/mod.rs | 4 +- .../src/tracers/validator/vm_latest/mod.rs | 4 +- .../validator/vm_refunds_enhancement/mod.rs | 4 +- .../validator/vm_virtual_blocks/mod.rs | 4 +- core/lib/vm_executor/src/oneshot/mock.rs | 41 +++++--- core/lib/vm_executor/src/oneshot/mod.rs | 99 ++++++++++--------- core/lib/vm_interface/src/executor.rs | 20 ++-- core/lib/vm_interface/src/types/tracer.rs | 21 ++-- .../src/execution_sandbox/execute.rs | 52 +++++----- .../src/execution_sandbox/validate.rs | 6 +- 12 files changed, 149 insertions(+), 114 deletions(-) diff --git a/core/lib/multivm/src/tracers/validator/vm_1_4_1/mod.rs b/core/lib/multivm/src/tracers/validator/vm_1_4_1/mod.rs index 5c245dcb75a..d1ddb2b44c8 100644 --- a/core/lib/multivm/src/tracers/validator/vm_1_4_1/mod.rs +++ b/core/lib/multivm/src/tracers/validator/vm_1_4_1/mod.rs @@ -88,7 +88,7 @@ impl ValidationTracer { Opcode::Context(context) => { match context { ContextOpcode::Meta => { - return Err(ViolatedValidationRule::TouchedUnallowedContext); + return Err(ViolatedValidationRule::TouchedDisallowedContext); } ContextOpcode::ErgsLeft => { // TODO (SMA-1168): implement the correct restrictions for the gas left opcode. @@ -102,7 +102,7 @@ impl ValidationTracer { let msg_sender = state.vm_local_state.callstack.current.msg_sender; if !self.is_allowed_storage_read(storage.clone(), this_address, key, msg_sender) { - return Err(ViolatedValidationRule::TouchedUnallowedStorageSlots( + return Err(ViolatedValidationRule::TouchedDisallowedStorageSlots( this_address, key, )); diff --git a/core/lib/multivm/src/tracers/validator/vm_1_4_2/mod.rs b/core/lib/multivm/src/tracers/validator/vm_1_4_2/mod.rs index edea8ddbadd..a51644ff9ea 100644 --- a/core/lib/multivm/src/tracers/validator/vm_1_4_2/mod.rs +++ b/core/lib/multivm/src/tracers/validator/vm_1_4_2/mod.rs @@ -88,7 +88,7 @@ impl ValidationTracer { Opcode::Context(context) => { match context { ContextOpcode::Meta => { - return Err(ViolatedValidationRule::TouchedUnallowedContext); + return Err(ViolatedValidationRule::TouchedDisallowedContext); } ContextOpcode::ErgsLeft => { // TODO (SMA-1168): implement the correct restrictions for the gas left opcode. @@ -102,7 +102,7 @@ impl ValidationTracer { let msg_sender = state.vm_local_state.callstack.current.msg_sender; if !self.is_allowed_storage_read(storage.clone(), this_address, key, msg_sender) { - return Err(ViolatedValidationRule::TouchedUnallowedStorageSlots( + return Err(ViolatedValidationRule::TouchedDisallowedStorageSlots( this_address, key, )); diff --git a/core/lib/multivm/src/tracers/validator/vm_boojum_integration/mod.rs b/core/lib/multivm/src/tracers/validator/vm_boojum_integration/mod.rs index ef032b5310a..7f9767a5e63 100644 --- a/core/lib/multivm/src/tracers/validator/vm_boojum_integration/mod.rs +++ b/core/lib/multivm/src/tracers/validator/vm_boojum_integration/mod.rs @@ -88,7 +88,7 @@ impl ValidationTracer { Opcode::Context(context) => { match context { ContextOpcode::Meta => { - return Err(ViolatedValidationRule::TouchedUnallowedContext); + return Err(ViolatedValidationRule::TouchedDisallowedContext); } ContextOpcode::ErgsLeft => { // TODO (SMA-1168): implement the correct restrictions for the gas left opcode. @@ -102,7 +102,7 @@ impl ValidationTracer { let msg_sender = state.vm_local_state.callstack.current.msg_sender; if !self.is_allowed_storage_read(storage.clone(), this_address, key, msg_sender) { - return Err(ViolatedValidationRule::TouchedUnallowedStorageSlots( + return Err(ViolatedValidationRule::TouchedDisallowedStorageSlots( this_address, key, )); diff --git a/core/lib/multivm/src/tracers/validator/vm_latest/mod.rs b/core/lib/multivm/src/tracers/validator/vm_latest/mod.rs index 68064902000..c206bd6fb2a 100644 --- a/core/lib/multivm/src/tracers/validator/vm_latest/mod.rs +++ b/core/lib/multivm/src/tracers/validator/vm_latest/mod.rs @@ -86,7 +86,7 @@ impl ValidationTracer { Opcode::Context(context) => { match context { ContextOpcode::Meta => { - return Err(ViolatedValidationRule::TouchedUnallowedContext); + return Err(ViolatedValidationRule::TouchedDisallowedContext); } ContextOpcode::ErgsLeft => { // TODO (SMA-1168): implement the correct restrictions for the gas left opcode. @@ -100,7 +100,7 @@ impl ValidationTracer { let msg_sender = state.vm_local_state.callstack.current.msg_sender; if !self.is_allowed_storage_read(storage.clone(), this_address, key, msg_sender) { - return Err(ViolatedValidationRule::TouchedUnallowedStorageSlots( + return Err(ViolatedValidationRule::TouchedDisallowedStorageSlots( this_address, key, )); diff --git a/core/lib/multivm/src/tracers/validator/vm_refunds_enhancement/mod.rs b/core/lib/multivm/src/tracers/validator/vm_refunds_enhancement/mod.rs index cd42221a42b..0badd7c5877 100644 --- a/core/lib/multivm/src/tracers/validator/vm_refunds_enhancement/mod.rs +++ b/core/lib/multivm/src/tracers/validator/vm_refunds_enhancement/mod.rs @@ -88,7 +88,7 @@ impl ValidationTracer { Opcode::Context(context) => { match context { ContextOpcode::Meta => { - return Err(ViolatedValidationRule::TouchedUnallowedContext); + return Err(ViolatedValidationRule::TouchedDisallowedContext); } ContextOpcode::ErgsLeft => { // TODO (SMA-1168): implement the correct restrictions for the gas left opcode. @@ -102,7 +102,7 @@ impl ValidationTracer { let msg_sender = state.vm_local_state.callstack.current.msg_sender; if !self.is_allowed_storage_read(storage.clone(), this_address, key, msg_sender) { - return Err(ViolatedValidationRule::TouchedUnallowedStorageSlots( + return Err(ViolatedValidationRule::TouchedDisallowedStorageSlots( this_address, key, )); diff --git a/core/lib/multivm/src/tracers/validator/vm_virtual_blocks/mod.rs b/core/lib/multivm/src/tracers/validator/vm_virtual_blocks/mod.rs index 45470422b00..86a639915c9 100644 --- a/core/lib/multivm/src/tracers/validator/vm_virtual_blocks/mod.rs +++ b/core/lib/multivm/src/tracers/validator/vm_virtual_blocks/mod.rs @@ -88,7 +88,7 @@ impl ValidationTracer { Opcode::Context(context) => { match context { ContextOpcode::Meta => { - return Err(ViolatedValidationRule::TouchedUnallowedContext); + return Err(ViolatedValidationRule::TouchedDisallowedContext); } ContextOpcode::ErgsLeft => { // TODO (SMA-1168): implement the correct restrictions for the gas left opcode. @@ -102,7 +102,7 @@ impl ValidationTracer { let msg_sender = state.vm_local_state.callstack.current.msg_sender; if !self.is_allowed_storage_read(storage.clone(), this_address, key, msg_sender) { - return Err(ViolatedValidationRule::TouchedUnallowedStorageSlots( + return Err(ViolatedValidationRule::TouchedDisallowedStorageSlots( this_address, key, )); diff --git a/core/lib/vm_executor/src/oneshot/mock.rs b/core/lib/vm_executor/src/oneshot/mock.rs index 642b2598767..3d09f768466 100644 --- a/core/lib/vm_executor/src/oneshot/mock.rs +++ b/core/lib/vm_executor/src/oneshot/mock.rs @@ -2,13 +2,13 @@ use std::fmt; use async_trait::async_trait; use zksync_multivm::interface::{ - executor::OneshotExecutor, + executor::{OneshotExecutor, TransactionValidator}, storage::ReadStorage, tracer::{ValidationError, ValidationParams}, ExecutionResult, OneshotEnv, OneshotTracingParams, OneshotTransactionExecutionResult, TxExecutionArgs, TxExecutionMode, VmExecutionResultAndLogs, }; -use zksync_types::Transaction; +use zksync_types::{l2::L2Tx, Transaction}; type TxResponseFn = dyn Fn(&Transaction, &OneshotEnv) -> VmExecutionResultAndLogs + Send + Sync; @@ -91,19 +91,6 @@ impl OneshotExecutor for MockOneshotExecutor where S: ReadStorage + Send + 'static, { - async fn validate_transaction( - &self, - _storage: S, - env: OneshotEnv, - args: TxExecutionArgs, - _validation_params: ValidationParams, - ) -> anyhow::Result> { - Ok(match self.mock_inspect(env, args).result { - ExecutionResult::Halt { reason } => Err(ValidationError::FailedTx(reason)), - ExecutionResult::Success { .. } | ExecutionResult::Revert { .. } => Ok(()), - }) - } - async fn inspect_transaction_with_bytecode_compression( &self, _storage: S, @@ -118,3 +105,27 @@ where }) } } + +#[async_trait] +impl TransactionValidator for MockOneshotExecutor +where + S: ReadStorage + Send + 'static, +{ + async fn validate_transaction( + &self, + _storage: S, + env: OneshotEnv, + tx: L2Tx, + _validation_params: ValidationParams, + ) -> anyhow::Result> { + Ok( + match self + .mock_inspect(env, TxExecutionArgs::for_validation(tx)) + .result + { + ExecutionResult::Halt { reason } => Err(ValidationError::FailedTx(reason)), + ExecutionResult::Success { .. } | ExecutionResult::Revert { .. } => Ok(()), + }, + ) + } +} diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs index b6e747f0e7b..eed4a564878 100644 --- a/core/lib/vm_executor/src/oneshot/mod.rs +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -10,7 +10,7 @@ use async_trait::async_trait; use once_cell::sync::OnceCell; use zksync_multivm::{ interface::{ - executor::OneshotExecutor, + executor::{OneshotExecutor, TransactionValidator}, storage::{ReadStorage, StoragePtr, StorageView, WriteStorage}, tracer::{ValidationError, ValidationParams}, ExecutionResult, OneshotEnv, OneshotTracingParams, OneshotTransactionExecutionResult, @@ -25,6 +25,7 @@ use zksync_multivm::{ 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}, AccountTreeId, Nonce, StorageKey, Transaction, SYSTEM_CONTEXT_ADDRESS, SYSTEM_CONTEXT_CURRENT_L2_BLOCK_INFO_POSITION, SYSTEM_CONTEXT_CURRENT_TX_ROLLING_HASH_POSITION, @@ -57,51 +58,6 @@ impl OneshotExecutor for MainOneshotExecutor where S: ReadStorage + Send + 'static, { - async fn validate_transaction( - &self, - storage: S, - env: OneshotEnv, - args: TxExecutionArgs, - validation_params: ValidationParams, - ) -> anyhow::Result> { - let missed_storage_invocation_limit = match env.system.execution_mode { - // storage accesses are not limited for tx validation - TxExecutionMode::VerifyExecute => usize::MAX, - TxExecutionMode::EthCall | TxExecutionMode::EstimateFee => { - self.missed_storage_invocation_limit - } - }; - - tokio::task::spawn_blocking(move || { - let (validation_tracer, mut validation_result) = - ValidationTracer::::new( - validation_params, - env.system.version.into(), - ); - let tracers = vec![ - validation_tracer.into_tracer_pointer(), - StorageInvocations::new(missed_storage_invocation_limit).into_tracer_pointer(), - ]; - - let executor = VmSandbox::new(storage, env, args); - let exec_result = executor.apply(|vm, transaction| { - vm.push_transaction(transaction); - vm.inspect(tracers.into(), VmExecutionMode::OneTx) - }); - let validation_result = Arc::make_mut(&mut validation_result) - .take() - .map_or(Ok(()), Err); - - match (exec_result.result, validation_result) { - (_, Err(violated_rule)) => Err(ValidationError::ViolatedRule(violated_rule)), - (ExecutionResult::Halt { reason }, _) => Err(ValidationError::FailedTx(reason)), - _ => Ok(()), - } - }) - .await - .context("VM execution panicked") - } - async fn inspect_transaction_with_bytecode_compression( &self, storage: S, @@ -150,6 +106,57 @@ where } } +#[async_trait] +impl TransactionValidator for MainOneshotExecutor +where + S: ReadStorage + Send + 'static, +{ + async fn validate_transaction( + &self, + storage: S, + env: OneshotEnv, + tx: L2Tx, + validation_params: ValidationParams, + ) -> anyhow::Result> { + let missed_storage_invocation_limit = match env.system.execution_mode { + // storage accesses are not limited for tx validation + TxExecutionMode::VerifyExecute => usize::MAX, + TxExecutionMode::EthCall | TxExecutionMode::EstimateFee => { + self.missed_storage_invocation_limit + } + }; + + tokio::task::spawn_blocking(move || { + let (validation_tracer, mut validation_result) = + ValidationTracer::::new( + validation_params, + env.system.version.into(), + ); + let tracers = vec![ + validation_tracer.into_tracer_pointer(), + StorageInvocations::new(missed_storage_invocation_limit).into_tracer_pointer(), + ]; + + let executor = VmSandbox::new(storage, env, TxExecutionArgs::for_validation(tx)); + let exec_result = executor.apply(|vm, transaction| { + vm.push_transaction(transaction); + vm.inspect(tracers.into(), VmExecutionMode::OneTx) + }); + let validation_result = Arc::make_mut(&mut validation_result) + .take() + .map_or(Ok(()), Err); + + match (exec_result.result, validation_result) { + (_, Err(violated_rule)) => Err(ValidationError::ViolatedRule(violated_rule)), + (ExecutionResult::Halt { reason }, _) => Err(ValidationError::FailedTx(reason)), + _ => Ok(()), + } + }) + .await + .context("VM execution panicked") + } +} + #[derive(Debug)] struct VmSandbox { vm: Box>, diff --git a/core/lib/vm_interface/src/executor.rs b/core/lib/vm_interface/src/executor.rs index 6a5a5ea5158..c0985bd6e38 100644 --- a/core/lib/vm_interface/src/executor.rs +++ b/core/lib/vm_interface/src/executor.rs @@ -3,7 +3,7 @@ use std::fmt; use async_trait::async_trait; -use zksync_types::Transaction; +use zksync_types::{l2::L2Tx, Transaction}; use crate::{ storage::{ReadStorage, StorageView}, @@ -48,19 +48,23 @@ pub trait BatchExecutor: 'static + Send + fmt::Debug { /// VM executor capable of executing isolated transactions / calls (as opposed to [batch execution](BatchExecutor)). #[async_trait] pub trait OneshotExecutor { - async fn validate_transaction( + async fn inspect_transaction_with_bytecode_compression( &self, storage: S, env: OneshotEnv, args: TxExecutionArgs, - validation_params: ValidationParams, - ) -> anyhow::Result>; + tracing: OneshotTracingParams, + ) -> anyhow::Result; +} - async fn inspect_transaction_with_bytecode_compression( +/// VM executor capable of validating transactions. +#[async_trait] +pub trait TransactionValidator: OneshotExecutor { + async fn validate_transaction( &self, storage: S, env: OneshotEnv, - args: TxExecutionArgs, - tracing: OneshotTracingParams, - ) -> anyhow::Result; + tx: L2Tx, + validation_params: ValidationParams, + ) -> anyhow::Result>; } diff --git a/core/lib/vm_interface/src/types/tracer.rs b/core/lib/vm_interface/src/types/tracer.rs index e743f8e22af..ba07772c7f2 100644 --- a/core/lib/vm_interface/src/types/tracer.rs +++ b/core/lib/vm_interface/src/types/tracer.rs @@ -42,6 +42,7 @@ pub enum VmExecutionStopReason { TracerRequestedStop(TracerExecutionStopReason), } +/// Transaction validation parameters. #[derive(Debug, Clone)] pub struct ValidationParams { pub user_address: Address, @@ -58,26 +59,31 @@ pub struct ValidationParams { pub computational_gas_limit: u32, } +/// Rules that can be violated when validating a transaction. #[derive(Debug, Clone)] pub enum ViolatedValidationRule { - TouchedUnallowedStorageSlots(Address, U256), + /// The transaction touched disallowed storage slots during validation. + TouchedDisallowedStorageSlots(Address, U256), + /// The transaction called a contract without attached bytecode. CalledContractWithNoCode(Address), - TouchedUnallowedContext, + /// The transaction touched disallowed context. + TouchedDisallowedContext, + /// The transaction used too much gas during validation. TookTooManyComputationalGas(u32), } impl fmt::Display for ViolatedValidationRule { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - ViolatedValidationRule::TouchedUnallowedStorageSlots(contract, key) => write!( + ViolatedValidationRule::TouchedDisallowedStorageSlots(contract, key) => write!( f, - "Touched unallowed storage slots: address {contract:x}, key: {key:x}", + "Touched disallowed storage slots: address {contract:x}, key: {key:x}", ), ViolatedValidationRule::CalledContractWithNoCode(contract) => { write!(f, "Called contract with no code: {contract:x}") } - ViolatedValidationRule::TouchedUnallowedContext => { - write!(f, "Touched unallowed context") + ViolatedValidationRule::TouchedDisallowedContext => { + write!(f, "Touched disallowed context") } ViolatedValidationRule::TookTooManyComputationalGas(gas_limit) => { write!( @@ -89,9 +95,12 @@ impl fmt::Display for ViolatedValidationRule { } } +/// Errors returned when validating a transaction. #[derive(Debug)] pub enum ValidationError { + /// VM execution was halted during validation. FailedTx(Halt), + /// Transaction violated one of account validation rules. ViolatedRule(ViolatedValidationRule), } diff --git a/core/node/api_server/src/execution_sandbox/execute.rs b/core/node/api_server/src/execution_sandbox/execute.rs index aa6b2c4171a..e8953927068 100644 --- a/core/node/api_server/src/execution_sandbox/execute.rs +++ b/core/node/api_server/src/execution_sandbox/execute.rs @@ -3,13 +3,13 @@ use async_trait::async_trait; use zksync_dal::{Connection, Core}; use zksync_multivm::interface::{ - executor::OneshotExecutor, + executor::{OneshotExecutor, TransactionValidator}, storage::ReadStorage, tracer::{ValidationError, ValidationParams}, Call, OneshotEnv, OneshotTracingParams, OneshotTransactionExecutionResult, TransactionExecutionMetrics, TxExecutionArgs, VmExecutionResultAndLogs, }; -use zksync_types::api::state_override::StateOverride; +use zksync_types::{api::state_override::StateOverride, l2::L2Tx}; use zksync_vm_executor::oneshot::{MainOneshotExecutor, MockOneshotExecutor}; use super::{apply, storage::StorageWithOverrides, vm_metrics, BlockArgs, TxSetupArgs, VmPermit}; @@ -91,27 +91,6 @@ impl OneshotExecutor for TransactionExecutor where S: ReadStorage + Send + 'static, { - async fn validate_transaction( - &self, - storage: S, - env: OneshotEnv, - args: TxExecutionArgs, - validation_params: ValidationParams, - ) -> anyhow::Result> { - match self { - Self::Real(executor) => { - executor - .validate_transaction(storage, env, args, validation_params) - .await - } - Self::Mock(executor) => { - executor - .validate_transaction(storage, env, args, validation_params) - .await - } - } - } - async fn inspect_transaction_with_bytecode_compression( &self, storage: S, @@ -143,3 +122,30 @@ where } } } + +#[async_trait] +impl TransactionValidator for TransactionExecutor +where + S: ReadStorage + Send + 'static, +{ + async fn validate_transaction( + &self, + storage: S, + env: OneshotEnv, + tx: L2Tx, + validation_params: ValidationParams, + ) -> anyhow::Result> { + match self { + Self::Real(executor) => { + executor + .validate_transaction(storage, env, tx, validation_params) + .await + } + Self::Mock(executor) => { + executor + .validate_transaction(storage, env, tx, validation_params) + .await + } + } + } +} diff --git a/core/node/api_server/src/execution_sandbox/validate.rs b/core/node/api_server/src/execution_sandbox/validate.rs index 24ff1c61e44..da6452de218 100644 --- a/core/node/api_server/src/execution_sandbox/validate.rs +++ b/core/node/api_server/src/execution_sandbox/validate.rs @@ -4,9 +4,8 @@ use anyhow::Context as _; use tracing::Instrument; use zksync_dal::{Connection, Core, CoreDal}; use zksync_multivm::interface::{ - executor::OneshotExecutor, + executor::TransactionValidator, tracer::{ValidationError as RawValidationError, ValidationParams}, - TxExecutionArgs, }; use zksync_types::{ api::state_override::StateOverride, l2::L2Tx, Address, TRUSTED_ADDRESS_SLOTS, @@ -56,10 +55,9 @@ impl TransactionExecutor { apply::prepare_env_and_storage(connection, setup_args, &block_args).await?; let storage = StorageWithOverrides::new(storage, &StateOverride::default()); - let execution_args = TxExecutionArgs::for_validation(tx); let stage_latency = SANDBOX_METRICS.sandbox[&SandboxStage::Validation].start(); let validation_result = self - .validate_transaction(storage, env, execution_args, validation_params) + .validate_transaction(storage, env, tx, validation_params) .instrument(tracing::debug_span!("validation")) .await?; drop(vm_permit); From 60b03230552897596bf5f36924ae2d75748e8ff6 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 4 Sep 2024 15:22:49 +0300 Subject: [PATCH 05/11] Restore oneshot executor test --- core/lib/vm_executor/src/oneshot/mod.rs | 17 +++---- .../api_server/src/execution_sandbox/mod.rs | 20 ++++++++ .../api_server/src/execution_sandbox/tests.rs | 46 +++++++++++-------- 3 files changed, 54 insertions(+), 29 deletions(-) diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs index eed4a564878..07d4ea2070f 100644 --- a/core/lib/vm_executor/src/oneshot/mod.rs +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -118,13 +118,11 @@ where tx: L2Tx, validation_params: ValidationParams, ) -> anyhow::Result> { - let missed_storage_invocation_limit = match env.system.execution_mode { - // storage accesses are not limited for tx validation - TxExecutionMode::VerifyExecute => usize::MAX, - TxExecutionMode::EthCall | TxExecutionMode::EstimateFee => { - self.missed_storage_invocation_limit - } - }; + anyhow::ensure!( + env.system.execution_mode == TxExecutionMode::VerifyExecute, + "Unexpected execution mode for tx validation: {:?} (expected `VerifyExecute`)", + env.system.execution_mode + ); tokio::task::spawn_blocking(move || { let (validation_tracer, mut validation_result) = @@ -132,10 +130,7 @@ where validation_params, env.system.version.into(), ); - let tracers = vec![ - validation_tracer.into_tracer_pointer(), - StorageInvocations::new(missed_storage_invocation_limit).into_tracer_pointer(), - ]; + let tracers = vec![validation_tracer.into_tracer_pointer()]; let executor = VmSandbox::new(storage, env, TxExecutionArgs::for_validation(tx)); let exec_result = executor.apply(|vm, transaction| { diff --git a/core/node/api_server/src/execution_sandbox/mod.rs b/core/node/api_server/src/execution_sandbox/mod.rs index 3be3735c36d..cdf9d8783ca 100644 --- a/core/node/api_server/src/execution_sandbox/mod.rs +++ b/core/node/api_server/src/execution_sandbox/mod.rs @@ -163,6 +163,26 @@ pub(crate) struct TxSetupArgs { pub enforced_base_fee: Option, } +impl TxSetupArgs { + #[cfg(test)] + pub fn mock( + execution_mode: TxExecutionMode, + base_system_contracts: MultiVMBaseSystemContracts, + ) -> Self { + Self { + execution_mode, + operator_account: AccountTreeId::default(), + fee_input: BatchFeeInput::l1_pegged(55, 555), + base_system_contracts, + caches: PostgresStorageCaches::new(1, 1), + validation_computational_gas_limit: u32::MAX, + chain_id: L2ChainId::default(), + whitelisted_tokens_for_aa: vec![], + enforced_base_fee: None, + } + } +} + #[derive(Debug, Clone, Copy)] struct BlockStartInfoInner { info: PruningInfo, diff --git a/core/node/api_server/src/execution_sandbox/tests.rs b/core/node/api_server/src/execution_sandbox/tests.rs index b5602b3c48f..e49f975a410 100644 --- a/core/node/api_server/src/execution_sandbox/tests.rs +++ b/core/node/api_server/src/execution_sandbox/tests.rs @@ -2,10 +2,17 @@ use assert_matches::assert_matches; use zksync_dal::ConnectionPool; +use zksync_multivm::{ + interface::{executor::OneshotExecutor, OneshotTracingParams, TxExecutionArgs}, + utils::derive_base_fee_and_gas_per_pubdata, +}; use zksync_node_genesis::{insert_genesis_batch, GenesisParams}; -use zksync_node_test_utils::{create_l2_block, prepare_recovery_snapshot}; +use zksync_node_test_utils::{create_l2_block, create_l2_transaction, prepare_recovery_snapshot}; +use zksync_types::{api::state_override::StateOverride, ProtocolVersionId, Transaction}; +use zksync_vm_executor::oneshot::MainOneshotExecutor; use super::*; +use crate::{execution_sandbox::storage::StorageWithOverrides, tx_sender::ApiContracts}; #[tokio::test] async fn creating_block_args() { @@ -161,7 +168,6 @@ async fn creating_block_args_after_snapshot_recovery() { } } -/* FIXME: move to executor #[tokio::test] async fn instantiating_vm() { let pool = ConnectionPool::::test_pool().await; @@ -184,25 +190,29 @@ async fn instantiating_vm() { } async fn test_instantiating_vm(connection: Connection<'static, Core>, block_args: BlockArgs) { - let transaction = Transaction::from(create_l2_transaction(10, 100)); let estimate_gas_contracts = ApiContracts::load_from_disk().await.unwrap().estimate_gas; + let mut setup_args = TxSetupArgs::mock(TxExecutionMode::EstimateFee, estimate_gas_contracts); + let (base_fee, gas_per_pubdata) = derive_base_fee_and_gas_per_pubdata( + setup_args.fee_input, + ProtocolVersionId::latest().into(), + ); + setup_args.enforced_base_fee = Some(base_fee); + let mut transaction = create_l2_transaction(base_fee, gas_per_pubdata); + transaction.common_data.fee.gas_limit = 200_000.into(); + let transaction = Transaction::from(transaction); let execution_args = TxExecutionArgs::for_gas_estimate(transaction.clone()); - let (env, storage) = apply::prepare_env_and_storage( - connection, - TxSetupArgs::mock(TxExecutionMode::EstimateFee, estimate_gas_contracts), - &block_args, - ) - .await - .unwrap(); + let (env, storage) = apply::prepare_env_and_storage(connection, setup_args, &block_args) + .await + .unwrap(); let storage = StorageWithOverrides::new(storage, &StateOverride::default()); - tokio::task::spawn_blocking(move || { - VmSandbox::new(storage, env, execution_args).apply(|_, received_tx| { - assert_eq!(received_tx, transaction); - }); - }) - .await - .expect("VM execution panicked") + let tracing_params = OneshotTracingParams::default(); + let output = MainOneshotExecutor::new(usize::MAX) + .inspect_transaction_with_bytecode_compression(storage, env, execution_args, tracing_params) + .await + .unwrap(); + output.compression_result.unwrap(); + let tx_result = *output.tx_result; + assert!(!tx_result.result.is_failed(), "{tx_result:#?}"); } -*/ From 6d8866be527d502e41a23084c442fde509f6058b Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 4 Sep 2024 15:57:19 +0300 Subject: [PATCH 06/11] Test transaction validation --- .../api_server/src/execution_sandbox/tests.rs | 106 ++++++++++++++++-- .../src/execution_sandbox/validate.rs | 2 +- 2 files changed, 98 insertions(+), 10 deletions(-) diff --git a/core/node/api_server/src/execution_sandbox/tests.rs b/core/node/api_server/src/execution_sandbox/tests.rs index e49f975a410..35103779a49 100644 --- a/core/node/api_server/src/execution_sandbox/tests.rs +++ b/core/node/api_server/src/execution_sandbox/tests.rs @@ -1,18 +1,31 @@ //! Tests for the VM execution sandbox. +use std::collections::HashMap; + use assert_matches::assert_matches; +use test_casing::test_casing; use zksync_dal::ConnectionPool; use zksync_multivm::{ - interface::{executor::OneshotExecutor, OneshotTracingParams, TxExecutionArgs}, + interface::{ + executor::{OneshotExecutor, TransactionValidator}, + tracer::ValidationError, + Halt, OneshotTracingParams, TxExecutionArgs, + }, utils::derive_base_fee_and_gas_per_pubdata, }; use zksync_node_genesis::{insert_genesis_batch, GenesisParams}; -use zksync_node_test_utils::{create_l2_block, create_l2_transaction, prepare_recovery_snapshot}; -use zksync_types::{api::state_override::StateOverride, ProtocolVersionId, Transaction}; +use zksync_node_test_utils::{create_l2_block, prepare_recovery_snapshot}; +use zksync_types::{ + api::state_override::{OverrideAccount, StateOverride}, + fee::Fee, + l2::L2Tx, + transaction_request::PaymasterParams, + K256PrivateKey, Nonce, ProtocolVersionId, Transaction, U256, +}; use zksync_vm_executor::oneshot::MainOneshotExecutor; -use super::*; -use crate::{execution_sandbox::storage::StorageWithOverrides, tx_sender::ApiContracts}; +use super::{storage::StorageWithOverrides, *}; +use crate::tx_sender::ApiContracts; #[tokio::test] async fn creating_block_args() { @@ -169,7 +182,7 @@ async fn creating_block_args_after_snapshot_recovery() { } #[tokio::test] -async fn instantiating_vm() { +async fn estimating_gas() { let pool = ConnectionPool::::test_pool().await; let mut connection = pool.connection().await.unwrap(); insert_genesis_batch(&mut connection, &GenesisParams::mock()) @@ -197,9 +210,7 @@ async fn test_instantiating_vm(connection: Connection<'static, Core>, block_args ProtocolVersionId::latest().into(), ); setup_args.enforced_base_fee = Some(base_fee); - let mut transaction = create_l2_transaction(base_fee, gas_per_pubdata); - transaction.common_data.fee.gas_limit = 200_000.into(); - let transaction = Transaction::from(transaction); + let transaction = Transaction::from(create_transfer(base_fee, gas_per_pubdata)); let execution_args = TxExecutionArgs::for_gas_estimate(transaction.clone()); let (env, storage) = apply::prepare_env_and_storage(connection, setup_args, &block_args) @@ -216,3 +227,80 @@ async fn test_instantiating_vm(connection: Connection<'static, Core>, block_args let tx_result = *output.tx_result; assert!(!tx_result.result.is_failed(), "{tx_result:#?}"); } + +fn create_transfer(fee_per_gas: u64, gas_per_pubdata: u64) -> L2Tx { + let fee = Fee { + gas_limit: 200_000.into(), + max_fee_per_gas: fee_per_gas.into(), + max_priority_fee_per_gas: 0_u64.into(), + gas_per_pubdata_limit: gas_per_pubdata.into(), + }; + L2Tx::new_signed( + Address::random(), + vec![], + Nonce(0), + fee, + U256::zero(), + L2ChainId::default(), + &K256PrivateKey::random(), + vec![], + PaymasterParams::default(), + ) + .unwrap() +} + +#[test_casing(2, [false, true])] +#[tokio::test] +async fn validating_transaction(set_balance: bool) { + let pool = ConnectionPool::::test_pool().await; + let mut connection = pool.connection().await.unwrap(); + insert_genesis_batch(&mut connection, &GenesisParams::mock()) + .await + .unwrap(); + + let block_args = BlockArgs::pending(&mut connection).await.unwrap(); + + let call_contracts = ApiContracts::load_from_disk().await.unwrap().eth_call; + let mut setup_args = TxSetupArgs::mock(TxExecutionMode::VerifyExecute, call_contracts); + let (base_fee, gas_per_pubdata) = derive_base_fee_and_gas_per_pubdata( + setup_args.fee_input, + ProtocolVersionId::latest().into(), + ); + setup_args.enforced_base_fee = Some(base_fee); + let transaction = create_transfer(base_fee, gas_per_pubdata); + + let validation_params = + validate::get_validation_params(&mut connection, &transaction, u32::MAX, &[]) + .await + .unwrap(); + let (env, storage) = apply::prepare_env_and_storage(connection, setup_args, &block_args) + .await + .unwrap(); + let state_override = if set_balance { + let account_override = OverrideAccount { + balance: Some(U256::from(1) << 128), + ..OverrideAccount::default() + }; + StateOverride::new(HashMap::from([( + transaction.initiator_account(), + account_override, + )])) + } else { + StateOverride::default() + }; + let storage = StorageWithOverrides::new(storage, &state_override); + + let validation_result = MainOneshotExecutor::new(usize::MAX) + .validate_transaction(storage, env, transaction, validation_params) + .await + .unwrap(); + if set_balance { + validation_result.expect("validation failed"); + } else { + assert_matches!( + validation_result.unwrap_err(), + ValidationError::FailedTx(Halt::ValidationFailed(reason)) + if reason.to_string().contains("Not enough balance") + ); + } +} diff --git a/core/node/api_server/src/execution_sandbox/validate.rs b/core/node/api_server/src/execution_sandbox/validate.rs index da6452de218..e9087e608ee 100644 --- a/core/node/api_server/src/execution_sandbox/validate.rs +++ b/core/node/api_server/src/execution_sandbox/validate.rs @@ -71,7 +71,7 @@ impl TransactionExecutor { /// Some slots can be marked as "trusted". That is needed for slots which can not be /// trusted to change between validation and execution in general case, but /// sometimes we can safely rely on them to not change often. -async fn get_validation_params( +pub(super) async fn get_validation_params( connection: &mut Connection<'_, Core>, tx: &L2Tx, computational_gas_limit: u32, From 0ef39d4495a6c3233ee978a7bc09d340c6e3e0da Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 5 Sep 2024 10:51:56 +0300 Subject: [PATCH 07/11] Remove duplicate method --- core/node/api_server/src/execution_sandbox/apply.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/core/node/api_server/src/execution_sandbox/apply.rs b/core/node/api_server/src/execution_sandbox/apply.rs index 875b9500af1..0fbf8abc3dd 100644 --- a/core/node/api_server/src/execution_sandbox/apply.rs +++ b/core/node/api_server/src/execution_sandbox/apply.rs @@ -245,15 +245,6 @@ impl BlockArgs { ) } - fn is_estimate_like(&self) -> bool { - matches!( - self.block_id, - api::BlockId::Number(api::BlockNumber::Pending) - | api::BlockId::Number(api::BlockNumber::Latest) - | api::BlockId::Number(api::BlockNumber::Committed) - ) - } - pub(crate) async fn default_eth_call_gas( &self, connection: &mut Connection<'_, Core>, @@ -307,7 +298,7 @@ impl BlockArgs { .context("resolved L2 block disappeared from storage")? }; - let historical_fee_input = if !self.is_estimate_like() { + let historical_fee_input = if !self.resolves_to_latest_sealed_l2_block() { let l2_block_header = connection .blocks_dal() .get_l2_block_header(self.resolved_block_number) From aa70c3b24a32d8845faa92d28ebc6c0b01470078 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 5 Sep 2024 18:03:29 +0300 Subject: [PATCH 08/11] Restore execution latency metric --- core/lib/vm_executor/src/oneshot/mod.rs | 36 ++++++++++++++++--- .../src/execution_sandbox/execute.rs | 11 ++++-- .../src/execution_sandbox/vm_metrics.rs | 1 + 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/core/lib/vm_executor/src/oneshot/mod.rs b/core/lib/vm_executor/src/oneshot/mod.rs index 07d4ea2070f..cac8edfdfdf 100644 --- a/core/lib/vm_executor/src/oneshot/mod.rs +++ b/core/lib/vm_executor/src/oneshot/mod.rs @@ -41,6 +41,7 @@ mod mock; #[derive(Debug, Default)] pub struct MainOneshotExecutor { missed_storage_invocation_limit: usize, + execution_latency_histogram: Option<&'static vise::Histogram>, } impl MainOneshotExecutor { @@ -49,8 +50,17 @@ impl MainOneshotExecutor { pub fn new(missed_storage_invocation_limit: usize) -> Self { Self { missed_storage_invocation_limit, + execution_latency_histogram: None, } } + + /// Sets a histogram for measuring VM execution latency. + pub fn set_execution_latency_histogram( + &mut self, + histogram: &'static vise::Histogram, + ) { + self.execution_latency_histogram = Some(histogram); + } } #[async_trait] @@ -72,6 +82,7 @@ where self.missed_storage_invocation_limit } }; + let execution_latency_histogram = self.execution_latency_histogram; tokio::task::spawn_blocking(move || { let mut tracers = vec![]; @@ -83,7 +94,7 @@ where StorageInvocations::new(missed_storage_invocation_limit).into_tracer_pointer(), ); - let executor = VmSandbox::new(storage, env, args); + 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( @@ -123,6 +134,7 @@ where "Unexpected execution mode for tx validation: {:?} (expected `VerifyExecute`)", env.system.execution_mode ); + let execution_latency_histogram = self.execution_latency_histogram; tokio::task::spawn_blocking(move || { let (validation_tracer, mut validation_result) = @@ -132,7 +144,12 @@ where ); let tracers = vec![validation_tracer.into_tracer_pointer()]; - let executor = VmSandbox::new(storage, env, TxExecutionArgs::for_validation(tx)); + let executor = VmSandbox::new( + storage, + env, + TxExecutionArgs::for_validation(tx), + execution_latency_histogram, + ); let exec_result = executor.apply(|vm, transaction| { vm.push_transaction(transaction); vm.inspect(tracers.into(), VmExecutionMode::OneTx) @@ -157,11 +174,17 @@ struct VmSandbox { vm: Box>, storage_view: StoragePtr>, transaction: Transaction, + execution_latency_histogram: Option<&'static vise::Histogram>, } impl VmSandbox { /// This method is blocking. - pub fn new(storage: S, mut env: OneshotEnv, execution_args: TxExecutionArgs) -> Self { + fn new( + storage: S, + 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); @@ -187,6 +210,7 @@ impl VmSandbox { vm, storage_view, transaction: execution_args.transaction, + execution_latency_histogram, } } @@ -248,9 +272,13 @@ impl VmSandbox { self.transaction.nonce().unwrap_or(Nonce(0)) ); + let started_at = Instant::now(); let result = apply_fn(&mut *self.vm, self.transaction); - let vm_execution_took = Duration::ZERO; // FIXME: metric + 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, diff --git a/core/node/api_server/src/execution_sandbox/execute.rs b/core/node/api_server/src/execution_sandbox/execute.rs index e8953927068..c373e49afc5 100644 --- a/core/node/api_server/src/execution_sandbox/execute.rs +++ b/core/node/api_server/src/execution_sandbox/execute.rs @@ -12,7 +12,11 @@ use zksync_multivm::interface::{ use zksync_types::{api::state_override::StateOverride, l2::L2Tx}; use zksync_vm_executor::oneshot::{MainOneshotExecutor, MockOneshotExecutor}; -use super::{apply, storage::StorageWithOverrides, vm_metrics, BlockArgs, TxSetupArgs, VmPermit}; +use super::{ + apply, storage::StorageWithOverrides, vm_metrics, BlockArgs, TxSetupArgs, VmPermit, + SANDBOX_METRICS, +}; +use crate::execution_sandbox::vm_metrics::SandboxStage; #[derive(Debug, Clone)] pub(crate) struct TransactionExecutionOutput { @@ -36,7 +40,10 @@ pub(crate) enum TransactionExecutor { impl TransactionExecutor { pub fn real(missed_storage_invocation_limit: usize) -> Self { - Self::Real(MainOneshotExecutor::new(missed_storage_invocation_limit)) + let mut executor = MainOneshotExecutor::new(missed_storage_invocation_limit); + executor + .set_execution_latency_histogram(&SANDBOX_METRICS.sandbox[&SandboxStage::Execution]); + Self::Real(executor) } /// This method assumes that (block with number `resolved_block_number` is present in DB) diff --git a/core/node/api_server/src/execution_sandbox/vm_metrics.rs b/core/node/api_server/src/execution_sandbox/vm_metrics.rs index 89a311fbb85..cbfe7e90bd0 100644 --- a/core/node/api_server/src/execution_sandbox/vm_metrics.rs +++ b/core/node/api_server/src/execution_sandbox/vm_metrics.rs @@ -19,6 +19,7 @@ pub(super) enum SandboxStage { Initialization, ValidateInSandbox, Validation, + Execution, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, EncodeLabelValue, EncodeLabelSet)] From 276a243047bb4cfab0b8d76f10afb6ee28cf20ed Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Thu, 5 Sep 2024 18:14:05 +0300 Subject: [PATCH 09/11] Fix small nits --- core/lib/multivm/src/glue/tracers/mod.rs | 4 ++-- core/lib/vm_executor/src/oneshot/mock.rs | 7 ++++++- core/lib/vm_interface/src/executor.rs | 2 ++ core/node/api_server/src/web3/tests/vm.rs | 2 +- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/core/lib/multivm/src/glue/tracers/mod.rs b/core/lib/multivm/src/glue/tracers/mod.rs index 3b9d576f32a..bf2f67cae50 100644 --- a/core/lib/multivm/src/glue/tracers/mod.rs +++ b/core/lib/multivm/src/glue/tracers/mod.rs @@ -32,7 +32,7 @@ use crate::{interface::storage::WriteStorage, tracers::old::OldTracers, HistoryMode}; -pub type MultiVmTracerPointer = Box + Send>; +pub type MultiVmTracerPointer = Box>; pub trait MultiVMTracer: IntoLatestTracer @@ -45,7 +45,7 @@ pub trait MultiVMTracer: { fn into_tracer_pointer(self) -> MultiVmTracerPointer where - Self: Sized + Send + 'static, + Self: Sized + 'static, { Box::new(self) } diff --git a/core/lib/vm_executor/src/oneshot/mock.rs b/core/lib/vm_executor/src/oneshot/mock.rs index 3d09f768466..8f3a12603c1 100644 --- a/core/lib/vm_executor/src/oneshot/mock.rs +++ b/core/lib/vm_executor/src/oneshot/mock.rs @@ -12,6 +12,7 @@ use zksync_types::{l2::L2Tx, Transaction}; type TxResponseFn = dyn Fn(&Transaction, &OneshotEnv) -> VmExecutionResultAndLogs + Send + Sync; +/// Mock [`OneshotExecutor`] implementation. pub struct MockOneshotExecutor { call_responses: Box, tx_responses: Box, @@ -39,6 +40,7 @@ impl Default for MockOneshotExecutor { } impl MockOneshotExecutor { + /// Sets call response closure used by this executor. pub fn set_call_responses(&mut self, responses: F) where F: Fn(&Transaction, &OneshotEnv) -> ExecutionResult + 'static + Send + Sync, @@ -46,6 +48,8 @@ impl MockOneshotExecutor { self.call_responses = self.wrap_responses(responses); } + /// Sets transaction response closure used by this executor. The closure will be called both for transaction execution / validation, + /// and for gas estimation. pub fn set_tx_responses(&mut self, responses: F) where F: Fn(&Transaction, &OneshotEnv) -> ExecutionResult + 'static + Send + Sync, @@ -69,7 +73,8 @@ impl MockOneshotExecutor { ) } - pub fn set_tx_responses_with_logs(&mut self, responses: F) + /// Same as [`Self::set_tx_responses()`], but allows to customize returned VM logs etc. + pub fn set_full_tx_responses(&mut self, responses: F) where F: Fn(&Transaction, &OneshotEnv) -> VmExecutionResultAndLogs + 'static + Send + Sync, { diff --git a/core/lib/vm_interface/src/executor.rs b/core/lib/vm_interface/src/executor.rs index c0985bd6e38..119f975fecd 100644 --- a/core/lib/vm_interface/src/executor.rs +++ b/core/lib/vm_interface/src/executor.rs @@ -48,6 +48,7 @@ pub trait BatchExecutor: 'static + Send + fmt::Debug { /// VM executor capable of executing isolated transactions / calls (as opposed to [batch execution](BatchExecutor)). #[async_trait] pub trait OneshotExecutor { + /// Executes a transaction or call with optional tracers. async fn inspect_transaction_with_bytecode_compression( &self, storage: S, @@ -60,6 +61,7 @@ pub trait OneshotExecutor { /// VM executor capable of validating transactions. #[async_trait] pub trait TransactionValidator: OneshotExecutor { + /// Validates the provided transaction. async fn validate_transaction( &self, storage: S, diff --git a/core/node/api_server/src/web3/tests/vm.rs b/core/node/api_server/src/web3/tests/vm.rs index 6cd12ed8d14..d8d1a2c7768 100644 --- a/core/node/api_server/src/web3/tests/vm.rs +++ b/core/node/api_server/src/web3/tests/vm.rs @@ -328,7 +328,7 @@ impl HttpTest for SendTransactionWithDetailedOutputTest { total_log_queries_count: 0, }; - tx_executor.set_tx_responses_with_logs(move |tx, env| { + tx_executor.set_full_tx_responses(move |tx, env| { assert_eq!(tx.hash(), tx_bytes_and_hash.1); assert_eq!(env.l1_batch.first_l2_block.number, 1); From bac7677ed47ee648ae820e41860bb3cc034a40d9 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 10 Sep 2024 14:14:16 +0300 Subject: [PATCH 10/11] Update contracts --- contracts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts b/contracts index fd4aebcfe88..d3687694f71 160000 --- a/contracts +++ b/contracts @@ -1 +1 @@ -Subproject commit fd4aebcfe8833b26e096e87e142a5e7e4744f3fa +Subproject commit d3687694f71d83fa286b9c186b4c3ea173028f83 From 28ccb714ef13e4837d20ecebda91758ed0441a65 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Tue, 10 Sep 2024 14:25:49 +0300 Subject: [PATCH 11/11] Remove unnecessary deps --- Cargo.lock | 4 ---- core/node/api_server/src/execution_sandbox/mod.rs | 2 +- core/node/consensus/Cargo.toml | 4 ---- core/node/consensus/src/en.rs | 2 +- core/node/consensus/src/vm.rs | 7 ++++--- 5 files changed, 6 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 82379128a70..ff1e44348b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9056,7 +9056,6 @@ dependencies = [ "anyhow", "async-trait", "hex", - "jsonrpsee", "rand 0.8.5", "secrecy", "semver", @@ -9065,7 +9064,6 @@ dependencies = [ "thiserror", "tokio", "tracing", - "zksync_basic_types", "zksync_concurrency", "zksync_config", "zksync_consensus_bft", @@ -9080,7 +9078,6 @@ dependencies = [ "zksync_l1_contract_interface", "zksync_merkle_tree", "zksync_metadata_calculator", - "zksync_multivm", "zksync_node_api_server", "zksync_node_genesis", "zksync_node_sync", @@ -9088,7 +9085,6 @@ dependencies = [ "zksync_protobuf", "zksync_state", "zksync_state_keeper", - "zksync_storage", "zksync_system_constants", "zksync_test_account", "zksync_types", diff --git a/core/node/api_server/src/execution_sandbox/mod.rs b/core/node/api_server/src/execution_sandbox/mod.rs index 82af3e2d6f8..79c6123642c 100644 --- a/core/node/api_server/src/execution_sandbox/mod.rs +++ b/core/node/api_server/src/execution_sandbox/mod.rs @@ -12,7 +12,7 @@ use zksync_types::{ api, fee_model::BatchFeeInput, AccountTreeId, Address, L1BatchNumber, L2BlockNumber, L2ChainId, }; -pub use self::execute::TransactionExecutor; // FIXME: remove +pub use self::execute::TransactionExecutor; // FIXME (PLA-1018): remove use self::vm_metrics::SandboxStage; pub(super) use self::{ error::SandboxExecutionError, diff --git a/core/node/consensus/Cargo.toml b/core/node/consensus/Cargo.toml index ba52892584d..707bd957d81 100644 --- a/core/node/consensus/Cargo.toml +++ b/core/node/consensus/Cargo.toml @@ -11,7 +11,6 @@ keywords.workspace = true categories.workspace = true [dependencies] -zksync_basic_types.workspace = true zksync_config.workspace = true zksync_concurrency.workspace = true zksync_consensus_crypto.workspace = true @@ -35,9 +34,7 @@ zksync_utils.workspace = true zksync_web3_decl.workspace = true zksync_node_api_server.workspace = true zksync_state.workspace = true -zksync_storage.workspace = true zksync_vm_interface.workspace = true -zksync_multivm.workspace = true anyhow.workspace = true async-trait.workspace = true secrecy.workspace = true @@ -46,7 +43,6 @@ thiserror.workspace = true tracing.workspace = true hex.workspace = true tokio.workspace = true -jsonrpsee.workspace = true semver.workspace = true [dev-dependencies] diff --git a/core/node/consensus/src/en.rs b/core/node/consensus/src/en.rs index e1f10b8e4e5..cf7e4173e8d 100644 --- a/core/node/consensus/src/en.rs +++ b/core/node/consensus/src/en.rs @@ -1,7 +1,6 @@ use std::sync::Arc; use anyhow::Context as _; -use jsonrpsee::{core::ClientError, types::error::ErrorCode}; use zksync_concurrency::{ctx, error::Wrap as _, scope, time}; use zksync_consensus_executor::{self as executor, attestation}; use zksync_consensus_roles::{attester, validator}; @@ -12,6 +11,7 @@ use zksync_types::L2BlockNumber; use zksync_web3_decl::{ client::{DynClient, L2}, error::is_retriable, + jsonrpsee::{core::ClientError, types::error::ErrorCode}, namespaces::{EnNamespaceClient as _, EthNamespaceClient as _}, }; diff --git a/core/node/consensus/src/vm.rs b/core/node/consensus/src/vm.rs index bdbfc8e03fd..11b6b5c67e3 100644 --- a/core/node/consensus/src/vm.rs +++ b/core/node/consensus/src/vm.rs @@ -1,7 +1,6 @@ use anyhow::Context as _; use zksync_concurrency::{ctx, error::Wrap as _, scope}; use zksync_consensus_roles::attester; -use zksync_multivm::interface::TxExecutionMode; use zksync_node_api_server::{ execution_sandbox::{TransactionExecutor, TxSetupArgs, VmConcurrencyLimiter}, tx_sender::MultiVMBaseSystemContracts, @@ -11,7 +10,9 @@ use zksync_system_constants::DEFAULT_L2_TX_GAS_PER_PUBDATA_BYTE; use zksync_types::{ ethabi, fee::Fee, fee_model::BatchFeeInput, l2::L2Tx, AccountTreeId, L2ChainId, Nonce, U256, }; -use zksync_vm_interface::{ExecutionResult, OneshotTracingParams, TxExecutionArgs}; +use zksync_vm_interface::{ + ExecutionResult, OneshotTracingParams, TxExecutionArgs, TxExecutionMode, +}; use crate::{abi, storage::ConnectionPool}; @@ -46,7 +47,7 @@ impl VM { } } - // FIXME: switch to oneshot executor + // FIXME (PLA-1018): switch to oneshot executor pub async fn call( &self, ctx: &ctx::Ctx,