From 7178b55e75c019e8067af4fd6a771e0fe9cc9e1b Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 25 Sep 2024 17:49:49 +0300 Subject: [PATCH 01/11] Update `vm2` revision --- core/lib/multivm/src/versions/vm_fast/vm.rs | 72 +++++++++++---------- 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/core/lib/multivm/src/versions/vm_fast/vm.rs b/core/lib/multivm/src/versions/vm_fast/vm.rs index 36698de105c1..be332d0eda84 100644 --- a/core/lib/multivm/src/versions/vm_fast/vm.rs +++ b/core/lib/multivm/src/versions/vm_fast/vm.rs @@ -17,7 +17,7 @@ use zksync_types::{ use zksync_utils::{bytecode::hash_bytecode, h256_to_u256, u256_to_h256}; use zksync_vm2::{ interface::{CallframeInterface, HeapId, StateInterface, Tracer}, - ExecutionEnd, FatPointer, Program, Settings, VirtualMachine, + ExecutionEnd, FatPointer, Program, Settings, StorageSlot, VirtualMachine, }; use super::{ @@ -430,24 +430,27 @@ impl Vm { } let storage = &mut self.world.storage; - let diffs = self.inner.world_diff().get_storage_changes().map( - move |((address, key), (initial_value, final_value))| { - let storage_key = StorageKey::new(AccountTreeId::new(address), u256_to_h256(key)); - StateDiffRecord { - address, - key, - derived_key: - zk_evm_1_5_0::aux_structures::LogQuery::derive_final_address_for_params( - &address, &key, - ), - enumeration_index: storage - .get_enumeration_index(&storage_key) - .unwrap_or_default(), - initial_value: initial_value.unwrap_or_default(), - final_value, - } - }, - ); + let diffs = + self.inner + .world_diff() + .get_storage_changes() + .map(move |((address, key), change)| { + let storage_key = + StorageKey::new(AccountTreeId::new(address), u256_to_h256(key)); + StateDiffRecord { + address, + key, + derived_key: + zk_evm_1_5_0::aux_structures::LogQuery::derive_final_address_for_params( + &address, &key, + ), + enumeration_index: storage + .get_enumeration_index(&storage_key) + .unwrap_or_default(), + initial_value: change.before, + final_value: change.after, + } + }); diffs .filter(|diff| diff.address != L1_MESSENGER_ADDRESS) .collect() @@ -477,9 +480,9 @@ impl Vm { events, deduplicated_storage_logs: world_diff .get_storage_changes() - .map(|((address, key), (_, value))| StorageLog { + .map(|((address, key), change)| StorageLog { key: StorageKey::new(AccountTreeId::new(address), u256_to_h256(key)), - value: u256_to_h256(value), + value: u256_to_h256(change.after), kind: StorageLogKind::RepeatedWrite, // Initialness doesn't matter here }) .collect(), @@ -556,7 +559,7 @@ impl VmInterface for Vm { StorageLogKind::RepeatedWrite }, }, - previous_value: u256_to_h256(change.before.unwrap_or_default()), + previous_value: u256_to_h256(change.before), }) .collect(); let events = merge_events( @@ -744,20 +747,23 @@ impl World { } impl zksync_vm2::StorageInterface for World { - fn read_storage(&mut self, contract: H160, key: U256) -> Option { + fn read_storage(&mut self, contract: H160, key: U256) -> StorageSlot { let key = &StorageKey::new(AccountTreeId::new(contract), u256_to_h256(key)); - if self.storage.is_write_initial(key) { - None - } else { - Some(self.storage.read_value(key).as_bytes().into()) + let value = U256::from_big_endian(self.storage.read_value(key).as_bytes()); + let is_write_initial = self.storage.is_write_initial(key); + StorageSlot { + value, + is_write_initial, } } - fn cost_of_writing_storage(&mut self, initial_value: Option, new_value: U256) -> u32 { - let is_initial = initial_value.is_none(); - let initial_value = initial_value.unwrap_or_default(); + fn read_storage_value(&mut self, contract: H160, key: U256) -> U256 { + let key = &StorageKey::new(AccountTreeId::new(contract), u256_to_h256(key)); + U256::from_big_endian(self.storage.read_value(key).as_bytes()) + } - if initial_value == new_value { + fn cost_of_writing_storage(&mut self, initial_slot: StorageSlot, new_value: U256) -> u32 { + if initial_slot.value == new_value { return 0; } @@ -772,9 +778,9 @@ impl zksync_vm2::StorageInterface for World { // previous state to the new state, and the compressed value. The maximum for this is 33 bytes. // Total bytes for initial writes then becomes 65 bytes and repeated writes becomes 38 bytes. let compressed_value_size = - compress_with_best_strategy(initial_value, new_value).len() as u32; + compress_with_best_strategy(initial_slot.value, new_value).len() as u32; - if is_initial { + if initial_slot.is_write_initial { (BYTES_PER_DERIVED_KEY as u32) + compressed_value_size } else { (BYTES_PER_ENUMERATION_INDEX as u32) + compressed_value_size From e03a0e689eef4b8a41e274afbdca3231655de5c1 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 27 Sep 2024 14:10:24 +0300 Subject: [PATCH 02/11] Update `vm2` revision --- Cargo.lock | 8 ++++---- Cargo.toml | 2 +- prover/Cargo.lock | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 079793ab9838..4587f1834195 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10950,8 +10950,8 @@ dependencies = [ [[package]] name = "zksync_vm2" -version = "0.1.0" -source = "git+https://github.com/matter-labs/vm2.git?rev=74577d9be13b1bff9d1a712389731f669b179e47#74577d9be13b1bff9d1a712389731f669b179e47" +version = "0.2.1" +source = "git+https://github.com/matter-labs/vm2.git?rev=1340e55e5016e9b680ffc8afe65d50488d065ce9#1340e55e5016e9b680ffc8afe65d50488d065ce9" dependencies = [ "enum_dispatch", "primitive-types", @@ -10962,8 +10962,8 @@ dependencies = [ [[package]] name = "zksync_vm2_interface" -version = "0.1.0" -source = "git+https://github.com/matter-labs/vm2.git?rev=74577d9be13b1bff9d1a712389731f669b179e47#74577d9be13b1bff9d1a712389731f669b179e47" +version = "0.2.1" +source = "git+https://github.com/matter-labs/vm2.git?rev=1340e55e5016e9b680ffc8afe65d50488d065ce9#1340e55e5016e9b680ffc8afe65d50488d065ce9" dependencies = [ "primitive-types", ] diff --git a/Cargo.toml b/Cargo.toml index d5ccff6eb1cf..d42bf78abd11 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -227,7 +227,7 @@ zk_evm_1_4_1 = { package = "zk_evm", version = "0.141" } zk_evm_1_5_0 = { package = "zk_evm", version = "=0.150.5" } # New VM; pinned to a specific commit because of instability -zksync_vm2 = { git = "https://github.com/matter-labs/vm2.git", rev = "74577d9be13b1bff9d1a712389731f669b179e47" } +zksync_vm2 = { git = "https://github.com/matter-labs/vm2.git", rev = "1340e55e5016e9b680ffc8afe65d50488d065ce9" } # FIXME: use commit from main branch # Consensus dependencies. zksync_concurrency = "=0.3.0" diff --git a/prover/Cargo.lock b/prover/Cargo.lock index 4ea83108a42e..3b0bb6be5220 100644 --- a/prover/Cargo.lock +++ b/prover/Cargo.lock @@ -8122,8 +8122,8 @@ dependencies = [ [[package]] name = "zksync_vm2" -version = "0.1.0" -source = "git+https://github.com/matter-labs/vm2.git?rev=74577d9be13b1bff9d1a712389731f669b179e47#74577d9be13b1bff9d1a712389731f669b179e47" +version = "0.2.1" +source = "git+https://github.com/matter-labs/vm2.git?rev=1340e55e5016e9b680ffc8afe65d50488d065ce9#1340e55e5016e9b680ffc8afe65d50488d065ce9" dependencies = [ "enum_dispatch", "primitive-types", @@ -8134,8 +8134,8 @@ dependencies = [ [[package]] name = "zksync_vm2_interface" -version = "0.1.0" -source = "git+https://github.com/matter-labs/vm2.git?rev=74577d9be13b1bff9d1a712389731f669b179e47#74577d9be13b1bff9d1a712389731f669b179e47" +version = "0.2.1" +source = "git+https://github.com/matter-labs/vm2.git?rev=1340e55e5016e9b680ffc8afe65d50488d065ce9#1340e55e5016e9b680ffc8afe65d50488d065ce9" dependencies = [ "primitive-types", ] From e2b2721fe15582295a76d9a1c99f8d483a07e086 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 30 Sep 2024 16:34:09 +0300 Subject: [PATCH 03/11] Fix logs rollback in call mode --- .../src/versions/vm_fast/tests/rollbacks.rs | 37 ++++++++++++- core/lib/multivm/src/versions/vm_fast/vm.rs | 55 +++++++++++++++---- .../vm_latest/bootloader_state/utils.rs | 4 +- .../src/versions/vm_latest/tests/rollbacks.rs | 40 +++++++++++++- 4 files changed, 119 insertions(+), 17 deletions(-) diff --git a/core/lib/multivm/src/versions/vm_fast/tests/rollbacks.rs b/core/lib/multivm/src/versions/vm_fast/tests/rollbacks.rs index e7b3f2043385..6d29e97456f9 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/rollbacks.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/rollbacks.rs @@ -1,9 +1,12 @@ +use assert_matches::assert_matches; use ethabi::Token; use zksync_contracts::{get_loadnext_contract, test_contracts::LoadnextContractExecutionParams}; -use zksync_types::{Execute, U256}; +use zksync_types::{Address, Execute, U256}; +use zksync_vm_interface::VmInterfaceExt; use crate::{ - interface::TxExecutionMode, + interface::{ExecutionResult, TxExecutionMode}, + versions::testonly::ContractToDeploy, vm_fast::tests::{ tester::{DeployContractsTx, TransactionTestInfo, TxModifier, TxType, VmTesterBuilder}, utils::read_test_contract, @@ -142,3 +145,33 @@ fn test_vm_loadnext_rollbacks() { assert_eq!(result_without_rollbacks, result_with_rollbacks); } + +#[test] +fn rollback_in_call_mode() { + let counter_bytecode = read_test_contract(); + let counter_address = Address::repeat_byte(1); + + let mut vm = VmTesterBuilder::new() + .with_empty_in_memory_storage() + .with_execution_mode(TxExecutionMode::EthCall) + .with_custom_contracts(vec![ContractToDeploy::new( + counter_bytecode, + counter_address, + )]) + .with_random_rich_accounts(1) + .build(); + let account = &mut vm.rich_accounts[0]; + let tx = account.get_test_contract_transaction(counter_address, true, None, false, TxType::L2); + + let (compression_result, vm_result) = vm + .vm + .execute_transaction_with_bytecode_compression(tx, true); + compression_result.unwrap(); + assert_matches!( + vm_result.result, + ExecutionResult::Revert { output } + if output.to_string().contains("This method always reverts") + ); + // The new VM performs deduplication internally and doesn't record reads, hence this crude comparison. + assert_eq!(vm_result.logs.storage_logs, []); +} diff --git a/core/lib/multivm/src/versions/vm_fast/vm.rs b/core/lib/multivm/src/versions/vm_fast/vm.rs index be332d0eda84..dcd0407fdb14 100644 --- a/core/lib/multivm/src/versions/vm_fast/vm.rs +++ b/core/lib/multivm/src/versions/vm_fast/vm.rs @@ -58,6 +58,27 @@ const VM_VERSION: MultiVMSubversion = MultiVMSubversion::IncreasedBootloaderMemo type FullTracer = (Tr, CircuitsTracer); +#[derive(Debug)] +struct VmRunResult { + execution_result: ExecutionResult, + /// `true` if VM execution has terminated (as opposed to being stopped on a hook, e.g. when executing a single transaction + /// in a batch). Used for `execution_result == Revert { .. }` to understand whether VM logs should be reverted. + execution_ended: bool, + refunds: Refunds, +} + +impl VmRunResult { + fn should_ignore_vm_logs(&self) -> bool { + match &self.execution_result { + ExecutionResult::Success { .. } => false, + ExecutionResult::Halt { .. } => true, + // Logs generated during reverts should only be ignored if the revert has reached the root (bootloader) call frame, + // which is only possible with a playground bootloader. + ExecutionResult::Revert { .. } => self.execution_ended, + } + } +} + /// Fast VM wrapper. /// /// The wrapper is parametric by the storage and tracer types. Besides the [`Tracer`] trait, a tracer must have `'static` lifetime @@ -140,32 +161,38 @@ impl Vm { execution_mode: VmExecutionMode, tracer: &mut (Tr, CircuitsTracer), track_refunds: bool, - ) -> (ExecutionResult, Refunds) { + ) -> VmRunResult { let mut refunds = Refunds { gas_refunded: 0, operator_suggested_refund: 0, }; let mut last_tx_result = None; let mut pubdata_before = self.inner.pubdata() as u32; + let mut execution_ended = false; - let result = loop { + let execution_result = loop { let hook = match self.inner.run(&mut self.world, tracer) { ExecutionEnd::SuspendedOnHook(hook) => hook, - ExecutionEnd::ProgramFinished(output) => break ExecutionResult::Success { output }, + ExecutionEnd::ProgramFinished(output) => { + execution_ended = true; + break ExecutionResult::Success { output }; + } ExecutionEnd::Reverted(output) => { + execution_ended = true; break match TxRevertReason::parse_error(&output) { TxRevertReason::TxReverted(output) => ExecutionResult::Revert { output }, TxRevertReason::Halt(reason) => ExecutionResult::Halt { reason }, - } + }; } ExecutionEnd::Panicked => { + execution_ended = true; break ExecutionResult::Halt { reason: if self.gas_remaining() == 0 { Halt::BootloaderOutOfGas } else { Halt::VMPanic }, - } + }; } }; @@ -305,7 +332,11 @@ impl Vm { } }; - (result, refunds) + VmRunResult { + execution_result, + execution_ended, + refunds, + } } fn get_hook_params(&self) -> [U256; 3] { @@ -534,14 +565,16 @@ impl VmInterface for Vm { let gas_before = self.gas_remaining(); let mut full_tracer = (mem::take(tracer), CircuitsTracer::default()); - let (result, refunds) = self.run(execution_mode, &mut full_tracer, track_refunds); + let result = self.run(execution_mode, &mut full_tracer, track_refunds); *tracer = full_tracer.0; // place the tracer back - let ignore_world_diff = matches!(execution_mode, VmExecutionMode::OneTx) - && matches!(result, ExecutionResult::Halt { .. }); + let ignore_world_diff = + matches!(execution_mode, VmExecutionMode::OneTx) && result.should_ignore_vm_logs(); // If the execution is halted, the VM changes are expected to be rolled back by the caller. // Earlier VMs return empty execution logs in this case, so we follow this behavior. + // Likewise, if a revert has reached the bootloader frame (possible with `TxExecutionMode::EthCall`; otherwise, the bootloader catches reverts), + // old VMs revert all logs; the new VM doesn't do that automatically, so we recreate this behavior here. let logs = if ignore_world_diff { VmExecutionLogs::default() } else { @@ -590,7 +623,7 @@ impl VmInterface for Vm { let pubdata_after = self.inner.pubdata(); let gas_remaining = self.gas_remaining(); VmExecutionResultAndLogs { - result, + result: result.execution_result, logs, // TODO (PLA-936): Fill statistics; investigate whether they should be zeroed on `Halt` statistics: VmExecutionStatistics { @@ -603,7 +636,7 @@ impl VmInterface for Vm { pubdata_published: (pubdata_after - pubdata_before).max(0) as u32, circuit_statistic: full_tracer.1.circuit_statistic(), }, - refunds, + refunds: result.refunds, } } diff --git a/core/lib/multivm/src/versions/vm_latest/bootloader_state/utils.rs b/core/lib/multivm/src/versions/vm_latest/bootloader_state/utils.rs index 4931082d6daf..23c079202c1f 100644 --- a/core/lib/multivm/src/versions/vm_latest/bootloader_state/utils.rs +++ b/core/lib/multivm/src/versions/vm_latest/bootloader_state/utils.rs @@ -175,8 +175,8 @@ pub(super) fn assemble_tx_meta(execution_mode: TxExecutionMode, execute_tx: bool // Set 0 byte (execution mode) output[0] = match execution_mode { TxExecutionMode::VerifyExecute => 0x00, - TxExecutionMode::EstimateFee { .. } => 0x00, - TxExecutionMode::EthCall { .. } => 0x02, + TxExecutionMode::EstimateFee => 0x00, + TxExecutionMode::EthCall => 0x02, }; // Set 31 byte (marker for tx execution) diff --git a/core/lib/multivm/src/versions/vm_latest/tests/rollbacks.rs b/core/lib/multivm/src/versions/vm_latest/tests/rollbacks.rs index 880f189fd892..00a5d6494fe1 100644 --- a/core/lib/multivm/src/versions/vm_latest/tests/rollbacks.rs +++ b/core/lib/multivm/src/versions/vm_latest/tests/rollbacks.rs @@ -1,12 +1,14 @@ +use assert_matches::assert_matches; use ethabi::Token; use zksync_contracts::{get_loadnext_contract, test_contracts::LoadnextContractExecutionParams}; -use zksync_types::{get_nonce_key, Execute, U256}; +use zksync_types::{get_nonce_key, Address, Execute, U256}; use crate::{ interface::{ storage::WriteStorage, tracer::{TracerExecutionStatus, TracerExecutionStopReason}, - TxExecutionMode, VmExecutionMode, VmInterface, VmInterfaceExt, VmInterfaceHistoryEnabled, + ExecutionResult, TxExecutionMode, VmExecutionMode, VmInterface, VmInterfaceExt, + VmInterfaceHistoryEnabled, }, tracers::dynamic::vm_1_5_0::DynTracer, vm_latest::{ @@ -258,3 +260,37 @@ fn test_layered_rollback() { let result = vm.vm.execute(VmExecutionMode::OneTx); assert!(!result.result.is_failed(), "transaction must not fail"); } + +#[test] +fn rollback_in_call_mode() { + let counter_bytecode = read_test_contract(); + let counter_address = Address::repeat_byte(1); + + let mut vm = VmTesterBuilder::new(HistoryEnabled) + .with_empty_in_memory_storage() + .with_execution_mode(TxExecutionMode::EthCall) + .with_custom_contracts(vec![(counter_bytecode, counter_address, false)]) + .with_random_rich_accounts(1) + .build(); + let account = &mut vm.rich_accounts[0]; + let tx = account.get_test_contract_transaction(counter_address, true, None, false, TxType::L2); + + let (compression_result, vm_result) = vm + .vm + .execute_transaction_with_bytecode_compression(tx, true); + compression_result.unwrap(); + assert_matches!( + vm_result.result, + ExecutionResult::Revert { output } + if output.to_string().contains("This method always reverts") + ); + + let storage_logs = vm + .vm + .get_current_execution_state() + .deduplicated_storage_logs; + assert!( + storage_logs.iter().all(|log| !log.is_write()), + "{storage_logs:?}" + ); +} From bfba2c37e5af2ba22f64dafef5b8885b7ea9aeab Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 30 Sep 2024 17:37:29 +0300 Subject: [PATCH 04/11] Make `Debug` repr for `Transaction` non-panicking --- core/lib/types/src/lib.rs | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/core/lib/types/src/lib.rs b/core/lib/types/src/lib.rs index 86b2e3f03d51..67661eb14ada 100644 --- a/core/lib/types/src/lib.rs +++ b/core/lib/types/src/lib.rs @@ -5,7 +5,7 @@ #![allow(clippy::upper_case_acronyms, clippy::derive_partial_eq_without_eq)] -use std::{fmt, fmt::Debug}; +use std::fmt; use anyhow::Context as _; use fee::encoding_len; @@ -88,9 +88,16 @@ pub struct Transaction { pub raw_bytes: Option, } -impl std::fmt::Debug for Transaction { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_tuple("Transaction").field(&self.hash()).finish() +impl fmt::Debug for Transaction { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(hash) = self.hash_for_debugging() { + f.debug_tuple("Transaction").field(&hash).finish() + } else { + f.debug_struct("Transaction") + .field("initiator_account", &self.initiator_account()) + .field("nonce", &self.nonce()) + .finish() + } } } @@ -136,6 +143,15 @@ impl Transaction { } } + fn hash_for_debugging(&self) -> Option { + match &self.common_data { + ExecuteTransactionCommon::L1(data) => Some(data.hash()), + ExecuteTransactionCommon::L2(data) if data.input.is_some() => Some(data.hash()), + ExecuteTransactionCommon::L2(_) => None, + ExecuteTransactionCommon::ProtocolUpgrade(data) => Some(data.hash()), + } + } + /// Returns the account that initiated this transaction. pub fn initiator_account(&self) -> Address { match &self.common_data { From eafb81cc43eaad435fd79ce590f2ec01f9171b08 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 30 Sep 2024 17:38:00 +0300 Subject: [PATCH 05/11] Fix panicking in `ShadowVm` --- core/lib/vm_interface/src/utils/shadow.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/lib/vm_interface/src/utils/shadow.rs b/core/lib/vm_interface/src/utils/shadow.rs index 2819e54e9a7b..e1cfa9a1cc38 100644 --- a/core/lib/vm_interface/src/utils/shadow.rs +++ b/core/lib/vm_interface/src/utils/shadow.rs @@ -202,7 +202,8 @@ where tx: Transaction, with_compression: bool, ) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs) { - let tx_hash = tx.hash(); + let tx_repr = format!("{tx:?}"); // includes little data, so is OK to call proactively + let (main_bytecodes_result, main_tx_result) = self.main.inspect_transaction_with_bytecode_compression( main_tracer, @@ -224,7 +225,7 @@ where errors.check_results_match(&main_tx_result, &shadow_result.1); if let Err(err) = errors.into_result() { let ctx = format!( - "inspecting transaction {tx_hash:?}, with_compression={with_compression:?}" + "inspecting transaction {tx_repr}, with_compression={with_compression:?}" ); self.report(err.context(ctx)); } From 8b3d8721827d435b2fe34d604a85db0a3485cf56 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 2 Oct 2024 12:10:32 +0300 Subject: [PATCH 06/11] Check more fields in `ShadowVm` --- core/lib/vm_interface/src/utils/shadow.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/core/lib/vm_interface/src/utils/shadow.rs b/core/lib/vm_interface/src/utils/shadow.rs index e1cfa9a1cc38..d0b3697d5ea5 100644 --- a/core/lib/vm_interface/src/utils/shadow.rs +++ b/core/lib/vm_interface/src/utils/shadow.rs @@ -342,10 +342,25 @@ impl DivergenceErrors { &shadow_result.statistics.circuit_statistic, ); self.check_match( - "gas_remaining", + "statistics.pubdata_published", + &main_result.statistics.pubdata_published, + &shadow_result.statistics.pubdata_published, + ); + self.check_match( + "statistics.gas_remaining", &main_result.statistics.gas_remaining, &shadow_result.statistics.gas_remaining, ); + self.check_match( + "statistics.gas_used", + &main_result.statistics.gas_used, + &shadow_result.statistics.gas_used, + ); + self.check_match( + "statistics.computational_gas_used", + &main_result.statistics.computational_gas_used, + &shadow_result.statistics.computational_gas_used, + ); } fn check_match(&mut self, context: &str, main: &T, shadow: &T) { From 674928616d07caed44061af57b40435cb6061831 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 2 Oct 2024 12:11:23 +0300 Subject: [PATCH 07/11] Fix `pubdata_published` divergence ...and fill in `computational_gas_used` --- core/lib/multivm/src/versions/vm_fast/vm.rs | 27 +++++++++++++-------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/core/lib/multivm/src/versions/vm_fast/vm.rs b/core/lib/multivm/src/versions/vm_fast/vm.rs index dcd0407fdb14..0f04bfb91e5f 100644 --- a/core/lib/multivm/src/versions/vm_fast/vm.rs +++ b/core/lib/multivm/src/versions/vm_fast/vm.rs @@ -65,6 +65,10 @@ struct VmRunResult { /// in a batch). Used for `execution_result == Revert { .. }` to understand whether VM logs should be reverted. execution_ended: bool, refunds: Refunds, + /// This value is used in stats. It's defined in the old VM as the latest value used when computing refunds (see the refunds tracer for `vm_latest`). + /// This is **not** equal to the pubdata diff before and after VM execution; e.g., when executing a batch tip, + /// `pubdata_published` is always 0 (since no refunds are getting computed). + pubdata_published: u32, } impl VmRunResult { @@ -168,6 +172,7 @@ impl Vm { }; let mut last_tx_result = None; let mut pubdata_before = self.inner.pubdata() as u32; + let mut pubdata_published = 0; let mut execution_ended = false; let execution_result = loop { @@ -219,7 +224,8 @@ impl Vm { ) .as_u64(); - let pubdata_published = self.inner.pubdata() as u32; + let pubdata_after = self.inner.pubdata() as u32; + pubdata_published = pubdata_after.saturating_sub(pubdata_before); refunds.operator_suggested_refund = compute_refund( &self.batch_env, @@ -227,7 +233,7 @@ impl Vm { gas_spent_on_pubdata.as_u64(), tx_gas_limit, gas_per_pubdata_byte.low_u32(), - pubdata_published.saturating_sub(pubdata_before), + pubdata_published, self.bootloader_state .last_l2_block() .txs @@ -236,7 +242,7 @@ impl Vm { .hash, ); - pubdata_before = pubdata_published; + pubdata_before = pubdata_after; let refund_value = refunds.operator_suggested_refund; self.write_to_bootloader_heap([( OPERATOR_REFUNDS_OFFSET + current_tx_index, @@ -336,6 +342,7 @@ impl Vm { execution_result, execution_ended, refunds, + pubdata_published, } } @@ -561,7 +568,6 @@ impl VmInterface for Vm { } let start = self.inner.world_diff().snapshot(); - let pubdata_before = self.inner.pubdata(); let gas_before = self.gas_remaining(); let mut full_tracer = (mem::take(tracer), CircuitsTracer::default()); @@ -620,21 +626,22 @@ impl VmInterface for Vm { } }; - let pubdata_after = self.inner.pubdata(); let gas_remaining = self.gas_remaining(); + let gas_used = gas_before - gas_remaining; + VmExecutionResultAndLogs { result: result.execution_result, logs, // TODO (PLA-936): Fill statistics; investigate whether they should be zeroed on `Halt` statistics: VmExecutionStatistics { + gas_used: gas_used.into(), + gas_remaining, + computational_gas_used: gas_used, // since 1.5.0, this always has the same value as `gas_used` + pubdata_published: result.pubdata_published, + circuit_statistic: full_tracer.1.circuit_statistic(), contracts_used: 0, cycles_used: 0, - gas_used: (gas_before - gas_remaining).into(), - gas_remaining, - computational_gas_used: 0, total_log_queries: 0, - pubdata_published: (pubdata_after - pubdata_before).max(0) as u32, - circuit_statistic: full_tracer.1.circuit_statistic(), }, refunds: result.refunds, } From c5db1f4e33e11e14589d19ddd2a5f28b5634dba9 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 2 Oct 2024 12:24:35 +0300 Subject: [PATCH 08/11] Use `vm2` commit from main branch --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- prover/Cargo.lock | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4587f1834195..00209adfce5b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10951,7 +10951,7 @@ dependencies = [ [[package]] name = "zksync_vm2" version = "0.2.1" -source = "git+https://github.com/matter-labs/vm2.git?rev=1340e55e5016e9b680ffc8afe65d50488d065ce9#1340e55e5016e9b680ffc8afe65d50488d065ce9" +source = "git+https://github.com/matter-labs/vm2.git?rev=a233d44bbe61dc6a758a754c3b78fe4f83e56699#a233d44bbe61dc6a758a754c3b78fe4f83e56699" dependencies = [ "enum_dispatch", "primitive-types", @@ -10963,7 +10963,7 @@ dependencies = [ [[package]] name = "zksync_vm2_interface" version = "0.2.1" -source = "git+https://github.com/matter-labs/vm2.git?rev=1340e55e5016e9b680ffc8afe65d50488d065ce9#1340e55e5016e9b680ffc8afe65d50488d065ce9" +source = "git+https://github.com/matter-labs/vm2.git?rev=a233d44bbe61dc6a758a754c3b78fe4f83e56699#a233d44bbe61dc6a758a754c3b78fe4f83e56699" dependencies = [ "primitive-types", ] diff --git a/Cargo.toml b/Cargo.toml index d42bf78abd11..471229ca86c3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -227,7 +227,7 @@ zk_evm_1_4_1 = { package = "zk_evm", version = "0.141" } zk_evm_1_5_0 = { package = "zk_evm", version = "=0.150.5" } # New VM; pinned to a specific commit because of instability -zksync_vm2 = { git = "https://github.com/matter-labs/vm2.git", rev = "1340e55e5016e9b680ffc8afe65d50488d065ce9" } # FIXME: use commit from main branch +zksync_vm2 = { git = "https://github.com/matter-labs/vm2.git", rev = "a233d44bbe61dc6a758a754c3b78fe4f83e56699" } # Consensus dependencies. zksync_concurrency = "=0.3.0" diff --git a/prover/Cargo.lock b/prover/Cargo.lock index 3b0bb6be5220..2a0c44030518 100644 --- a/prover/Cargo.lock +++ b/prover/Cargo.lock @@ -8123,7 +8123,7 @@ dependencies = [ [[package]] name = "zksync_vm2" version = "0.2.1" -source = "git+https://github.com/matter-labs/vm2.git?rev=1340e55e5016e9b680ffc8afe65d50488d065ce9#1340e55e5016e9b680ffc8afe65d50488d065ce9" +source = "git+https://github.com/matter-labs/vm2.git?rev=a233d44bbe61dc6a758a754c3b78fe4f83e56699#a233d44bbe61dc6a758a754c3b78fe4f83e56699" dependencies = [ "enum_dispatch", "primitive-types", @@ -8135,7 +8135,7 @@ dependencies = [ [[package]] name = "zksync_vm2_interface" version = "0.2.1" -source = "git+https://github.com/matter-labs/vm2.git?rev=1340e55e5016e9b680ffc8afe65d50488d065ce9#1340e55e5016e9b680ffc8afe65d50488d065ce9" +source = "git+https://github.com/matter-labs/vm2.git?rev=a233d44bbe61dc6a758a754c3b78fe4f83e56699#a233d44bbe61dc6a758a754c3b78fe4f83e56699" dependencies = [ "primitive-types", ] From 7a5f1404a659d25c839a13543dd6d31c373a9d81 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 2 Oct 2024 13:07:59 +0300 Subject: [PATCH 09/11] Brush up VM code --- .../src/versions/vm_1_4_1/bootloader_state/utils.rs | 4 ++-- .../src/versions/vm_1_4_2/bootloader_state/utils.rs | 4 ++-- .../vm_boojum_integration/bootloader_state/utils.rs | 4 ++-- .../src/versions/vm_fast/bootloader_state/utils.rs | 4 ++-- core/lib/multivm/src/versions/vm_fast/vm.rs | 13 ++++++------- .../bootloader_state/utils.rs | 4 ++-- .../vm_virtual_blocks/bootloader_state/utils.rs | 4 ++-- 7 files changed, 18 insertions(+), 19 deletions(-) diff --git a/core/lib/multivm/src/versions/vm_1_4_1/bootloader_state/utils.rs b/core/lib/multivm/src/versions/vm_1_4_1/bootloader_state/utils.rs index 393eb043cb76..1acf75b27e1b 100644 --- a/core/lib/multivm/src/versions/vm_1_4_1/bootloader_state/utils.rs +++ b/core/lib/multivm/src/versions/vm_1_4_1/bootloader_state/utils.rs @@ -167,8 +167,8 @@ pub(super) fn assemble_tx_meta(execution_mode: TxExecutionMode, execute_tx: bool // Set 0 byte (execution mode) output[0] = match execution_mode { TxExecutionMode::VerifyExecute => 0x00, - TxExecutionMode::EstimateFee { .. } => 0x00, - TxExecutionMode::EthCall { .. } => 0x02, + TxExecutionMode::EstimateFee => 0x00, + TxExecutionMode::EthCall => 0x02, }; // Set 31 byte (marker for tx execution) diff --git a/core/lib/multivm/src/versions/vm_1_4_2/bootloader_state/utils.rs b/core/lib/multivm/src/versions/vm_1_4_2/bootloader_state/utils.rs index 600ab83bf484..182f6eff4414 100644 --- a/core/lib/multivm/src/versions/vm_1_4_2/bootloader_state/utils.rs +++ b/core/lib/multivm/src/versions/vm_1_4_2/bootloader_state/utils.rs @@ -167,8 +167,8 @@ pub(super) fn assemble_tx_meta(execution_mode: TxExecutionMode, execute_tx: bool // Set 0 byte (execution mode) output[0] = match execution_mode { TxExecutionMode::VerifyExecute => 0x00, - TxExecutionMode::EstimateFee { .. } => 0x00, - TxExecutionMode::EthCall { .. } => 0x02, + TxExecutionMode::EstimateFee => 0x00, + TxExecutionMode::EthCall => 0x02, }; // Set 31 byte (marker for tx execution) diff --git a/core/lib/multivm/src/versions/vm_boojum_integration/bootloader_state/utils.rs b/core/lib/multivm/src/versions/vm_boojum_integration/bootloader_state/utils.rs index 1a1c620c2b26..c97d3ff30e49 100644 --- a/core/lib/multivm/src/versions/vm_boojum_integration/bootloader_state/utils.rs +++ b/core/lib/multivm/src/versions/vm_boojum_integration/bootloader_state/utils.rs @@ -167,8 +167,8 @@ pub(super) fn assemble_tx_meta(execution_mode: TxExecutionMode, execute_tx: bool // Set 0 byte (execution mode) output[0] = match execution_mode { TxExecutionMode::VerifyExecute => 0x00, - TxExecutionMode::EstimateFee { .. } => 0x00, - TxExecutionMode::EthCall { .. } => 0x02, + TxExecutionMode::EstimateFee => 0x00, + TxExecutionMode::EthCall => 0x02, }; // Set 31 byte (marker for tx execution) diff --git a/core/lib/multivm/src/versions/vm_fast/bootloader_state/utils.rs b/core/lib/multivm/src/versions/vm_fast/bootloader_state/utils.rs index f280f56a828a..770f232019bf 100644 --- a/core/lib/multivm/src/versions/vm_fast/bootloader_state/utils.rs +++ b/core/lib/multivm/src/versions/vm_fast/bootloader_state/utils.rs @@ -171,8 +171,8 @@ pub(super) fn assemble_tx_meta(execution_mode: TxExecutionMode, execute_tx: bool // Set 0 byte (execution mode) output[0] = match execution_mode { TxExecutionMode::VerifyExecute => 0x00, - TxExecutionMode::EstimateFee { .. } => 0x00, - TxExecutionMode::EthCall { .. } => 0x02, + TxExecutionMode::EstimateFee => 0x00, + TxExecutionMode::EthCall => 0x02, }; // Set 31 byte (marker for tx execution) diff --git a/core/lib/multivm/src/versions/vm_fast/vm.rs b/core/lib/multivm/src/versions/vm_fast/vm.rs index 0f04bfb91e5f..2d7ba9778e35 100644 --- a/core/lib/multivm/src/versions/vm_fast/vm.rs +++ b/core/lib/multivm/src/versions/vm_fast/vm.rs @@ -1,6 +1,8 @@ use std::{collections::HashMap, fmt, mem}; -use zk_evm_1_5_0::zkevm_opcode_defs::system_params::INITIAL_FRAME_FORMAL_EH_LOCATION; +use zk_evm_1_5_0::{ + aux_structures::LogQuery, zkevm_opcode_defs::system_params::INITIAL_FRAME_FORMAL_EH_LOCATION, +}; use zksync_contracts::SystemContractCode; use zksync_types::{ l1::is_l1_tx_type, @@ -67,7 +69,7 @@ struct VmRunResult { refunds: Refunds, /// This value is used in stats. It's defined in the old VM as the latest value used when computing refunds (see the refunds tracer for `vm_latest`). /// This is **not** equal to the pubdata diff before and after VM execution; e.g., when executing a batch tip, - /// `pubdata_published` is always 0 (since no refunds are getting computed). + /// `pubdata_published` is always 0 (since no refunds are computed). pubdata_published: u32, } @@ -77,7 +79,7 @@ impl VmRunResult { ExecutionResult::Success { .. } => false, ExecutionResult::Halt { .. } => true, // Logs generated during reverts should only be ignored if the revert has reached the root (bootloader) call frame, - // which is only possible with a playground bootloader. + // which is only possible with `TxExecutionMode::EthCall`. ExecutionResult::Revert { .. } => self.execution_ended, } } @@ -478,10 +480,7 @@ impl Vm { StateDiffRecord { address, key, - derived_key: - zk_evm_1_5_0::aux_structures::LogQuery::derive_final_address_for_params( - &address, &key, - ), + derived_key: LogQuery::derive_final_address_for_params(&address, &key), enumeration_index: storage .get_enumeration_index(&storage_key) .unwrap_or_default(), diff --git a/core/lib/multivm/src/versions/vm_refunds_enhancement/bootloader_state/utils.rs b/core/lib/multivm/src/versions/vm_refunds_enhancement/bootloader_state/utils.rs index 7bd488f90a9c..14c895d7a0b4 100644 --- a/core/lib/multivm/src/versions/vm_refunds_enhancement/bootloader_state/utils.rs +++ b/core/lib/multivm/src/versions/vm_refunds_enhancement/bootloader_state/utils.rs @@ -133,8 +133,8 @@ pub(super) fn assemble_tx_meta(execution_mode: TxExecutionMode, execute_tx: bool // Set 0 byte (execution mode) output[0] = match execution_mode { TxExecutionMode::VerifyExecute => 0x00, - TxExecutionMode::EstimateFee { .. } => 0x00, - TxExecutionMode::EthCall { .. } => 0x02, + TxExecutionMode::EstimateFee => 0x00, + TxExecutionMode::EthCall => 0x02, }; // Set 31 byte (marker for tx execution) diff --git a/core/lib/multivm/src/versions/vm_virtual_blocks/bootloader_state/utils.rs b/core/lib/multivm/src/versions/vm_virtual_blocks/bootloader_state/utils.rs index 2ccedcc6aa94..3e2474835fa4 100644 --- a/core/lib/multivm/src/versions/vm_virtual_blocks/bootloader_state/utils.rs +++ b/core/lib/multivm/src/versions/vm_virtual_blocks/bootloader_state/utils.rs @@ -133,8 +133,8 @@ pub(super) fn assemble_tx_meta(execution_mode: TxExecutionMode, execute_tx: bool // Set 0 byte (execution mode) output[0] = match execution_mode { TxExecutionMode::VerifyExecute => 0x00, - TxExecutionMode::EstimateFee { .. } => 0x00, - TxExecutionMode::EthCall { .. } => 0x02, + TxExecutionMode::EstimateFee => 0x00, + TxExecutionMode::EthCall => 0x02, }; // Set 31 byte (marker for tx execution) From 2fc3328cf5eacf14230a49c86d589914594c122c Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 2 Oct 2024 13:33:46 +0300 Subject: [PATCH 10/11] Remove `record_vm_memory_metrics` from VM interface --- core/lib/multivm/src/versions/vm_1_3_2/vm.rs | 36 ++++++++++--------- .../vm_1_4_1/implementation/statistics.rs | 2 +- core/lib/multivm/src/versions/vm_1_4_1/vm.rs | 5 --- .../vm_1_4_2/implementation/statistics.rs | 2 +- core/lib/multivm/src/versions/vm_1_4_2/vm.rs | 5 --- .../implementation/statistics.rs | 2 +- .../src/versions/vm_boojum_integration/vm.rs | 5 --- core/lib/multivm/src/versions/vm_fast/vm.rs | 8 ++--- .../vm_latest/implementation/statistics.rs | 2 +- core/lib/multivm/src/versions/vm_latest/vm.rs | 6 +--- core/lib/multivm/src/versions/vm_m5/vm.rs | 17 +++------ core/lib/multivm/src/versions/vm_m6/vm.rs | 34 +++++++++--------- .../implementation/statistics.rs | 2 +- .../src/versions/vm_refunds_enhancement/vm.rs | 5 --- .../implementation/statistics.rs | 2 +- .../src/versions/vm_virtual_blocks/vm.rs | 5 --- core/lib/multivm/src/vm_instance.rs | 13 +++---- .../src/types/outputs/statistic.rs | 3 +- core/lib/vm_interface/src/utils/dump.rs | 6 +--- core/lib/vm_interface/src/utils/shadow.rs | 6 +--- core/lib/vm_interface/src/vm.rs | 5 +-- 21 files changed, 59 insertions(+), 112 deletions(-) diff --git a/core/lib/multivm/src/versions/vm_1_3_2/vm.rs b/core/lib/multivm/src/versions/vm_1_3_2/vm.rs index 5692f103da3d..89196788a762 100644 --- a/core/lib/multivm/src/versions/vm_1_3_2/vm.rs +++ b/core/lib/multivm/src/versions/vm_1_3_2/vm.rs @@ -22,6 +22,25 @@ pub struct Vm { pub(crate) system_env: SystemEnv, } +impl Vm { + pub(crate) fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { + VmMemoryMetrics { + event_sink_inner: self.vm.state.event_sink.get_size(), + event_sink_history: self.vm.state.event_sink.get_history_size(), + memory_inner: self.vm.state.memory.get_size(), + memory_history: self.vm.state.memory.get_history_size(), + decommittment_processor_inner: self.vm.state.decommittment_processor.get_size(), + decommittment_processor_history: self + .vm + .state + .decommittment_processor + .get_history_size(), + storage_inner: self.vm.state.storage.get_size(), + storage_history: self.vm.state.storage.get_history_size(), + } + } +} + impl VmInterface for Vm { type TracerDispatcher = TracerDispatcher; @@ -160,23 +179,6 @@ impl VmInterface for Vm { } } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - VmMemoryMetrics { - event_sink_inner: self.vm.state.event_sink.get_size(), - event_sink_history: self.vm.state.event_sink.get_history_size(), - memory_inner: self.vm.state.memory.get_size(), - memory_history: self.vm.state.memory.get_history_size(), - decommittment_processor_inner: self.vm.state.decommittment_processor.get_size(), - decommittment_processor_history: self - .vm - .state - .decommittment_processor - .get_history_size(), - storage_inner: self.vm.state.storage.get_size(), - storage_history: self.vm.state.storage.get_history_size(), - } - } - fn finish_batch(&mut self) -> FinishedL1Batch { self.vm .execute_till_block_end( diff --git a/core/lib/multivm/src/versions/vm_1_4_1/implementation/statistics.rs b/core/lib/multivm/src/versions/vm_1_4_1/implementation/statistics.rs index 71ae20d44061..3a3b22ea2460 100644 --- a/core/lib/multivm/src/versions/vm_1_4_1/implementation/statistics.rs +++ b/core/lib/multivm/src/versions/vm_1_4_1/implementation/statistics.rs @@ -57,7 +57,7 @@ impl Vm { } /// Returns the info about all oracles' sizes. - pub(crate) fn record_vm_memory_metrics_inner(&self) -> VmMemoryMetrics { + pub(crate) fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { VmMemoryMetrics { event_sink_inner: self.state.event_sink.get_size(), event_sink_history: self.state.event_sink.get_history_size(), diff --git a/core/lib/multivm/src/versions/vm_1_4_1/vm.rs b/core/lib/multivm/src/versions/vm_1_4_1/vm.rs index 68c8e92a03a1..4122ee94e66a 100644 --- a/core/lib/multivm/src/versions/vm_1_4_1/vm.rs +++ b/core/lib/multivm/src/versions/vm_1_4_1/vm.rs @@ -11,7 +11,6 @@ use crate::{ BytecodeCompressionError, BytecodeCompressionResult, CurrentExecutionState, FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceHistoryEnabled, - VmMemoryMetrics, }, utils::events::extract_l2tol1logs_from_l1_messenger, vm_1_4_1::{ @@ -124,10 +123,6 @@ impl VmInterface for Vm { } } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - self.record_vm_memory_metrics_inner() - } - fn finish_batch(&mut self) -> FinishedL1Batch { let result = self.inspect(&mut TracerDispatcher::default(), VmExecutionMode::Batch); let execution_state = self.get_current_execution_state(); diff --git a/core/lib/multivm/src/versions/vm_1_4_2/implementation/statistics.rs b/core/lib/multivm/src/versions/vm_1_4_2/implementation/statistics.rs index 92a2eaa650c1..754b8476182f 100644 --- a/core/lib/multivm/src/versions/vm_1_4_2/implementation/statistics.rs +++ b/core/lib/multivm/src/versions/vm_1_4_2/implementation/statistics.rs @@ -57,7 +57,7 @@ impl Vm { } /// Returns the info about all oracles' sizes. - pub(crate) fn record_vm_memory_metrics_inner(&self) -> VmMemoryMetrics { + pub(crate) fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { VmMemoryMetrics { event_sink_inner: self.state.event_sink.get_size(), event_sink_history: self.state.event_sink.get_history_size(), diff --git a/core/lib/multivm/src/versions/vm_1_4_2/vm.rs b/core/lib/multivm/src/versions/vm_1_4_2/vm.rs index d6e1fbc68a8f..fe2015debd2b 100644 --- a/core/lib/multivm/src/versions/vm_1_4_2/vm.rs +++ b/core/lib/multivm/src/versions/vm_1_4_2/vm.rs @@ -13,7 +13,6 @@ use crate::{ BytecodeCompressionError, BytecodeCompressionResult, CurrentExecutionState, FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceHistoryEnabled, - VmMemoryMetrics, }, utils::events::extract_l2tol1logs_from_l1_messenger, vm_1_4_2::{ @@ -126,10 +125,6 @@ impl VmInterface for Vm { } } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - self.record_vm_memory_metrics_inner() - } - fn finish_batch(&mut self) -> FinishedL1Batch { let result = self.inspect(&mut TracerDispatcher::default(), VmExecutionMode::Batch); let execution_state = self.get_current_execution_state(); diff --git a/core/lib/multivm/src/versions/vm_boojum_integration/implementation/statistics.rs b/core/lib/multivm/src/versions/vm_boojum_integration/implementation/statistics.rs index 46f8bc2f400b..015d5acd340b 100644 --- a/core/lib/multivm/src/versions/vm_boojum_integration/implementation/statistics.rs +++ b/core/lib/multivm/src/versions/vm_boojum_integration/implementation/statistics.rs @@ -57,7 +57,7 @@ impl Vm { } /// Returns the info about all oracles' sizes. - pub(crate) fn record_vm_memory_metrics_inner(&self) -> VmMemoryMetrics { + pub(crate) fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { VmMemoryMetrics { event_sink_inner: self.state.event_sink.get_size(), event_sink_history: self.state.event_sink.get_history_size(), diff --git a/core/lib/multivm/src/versions/vm_boojum_integration/vm.rs b/core/lib/multivm/src/versions/vm_boojum_integration/vm.rs index 17ce8365a0aa..ebc0a511d203 100644 --- a/core/lib/multivm/src/versions/vm_boojum_integration/vm.rs +++ b/core/lib/multivm/src/versions/vm_boojum_integration/vm.rs @@ -11,7 +11,6 @@ use crate::{ BytecodeCompressionError, BytecodeCompressionResult, CurrentExecutionState, FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceHistoryEnabled, - VmMemoryMetrics, }, utils::events::extract_l2tol1logs_from_l1_messenger, vm_boojum_integration::{ @@ -125,10 +124,6 @@ impl VmInterface for Vm { } } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - self.record_vm_memory_metrics_inner() - } - fn finish_batch(&mut self) -> FinishedL1Batch { let result = self.inspect(&mut TracerDispatcher::default(), VmExecutionMode::Batch); let execution_state = self.get_current_execution_state(); diff --git a/core/lib/multivm/src/versions/vm_fast/vm.rs b/core/lib/multivm/src/versions/vm_fast/vm.rs index 2d7ba9778e35..c6b3f4411e60 100644 --- a/core/lib/multivm/src/versions/vm_fast/vm.rs +++ b/core/lib/multivm/src/versions/vm_fast/vm.rs @@ -37,8 +37,8 @@ use crate::{ BytecodeCompressionError, BytecodeCompressionResult, CurrentExecutionState, ExecutionResult, FinishedL1Batch, Halt, L1BatchEnv, L2BlockEnv, Refunds, SystemEnv, TxRevertReason, VmEvent, VmExecutionLogs, VmExecutionMode, VmExecutionResultAndLogs, - VmExecutionStatistics, VmFactory, VmInterface, VmInterfaceHistoryEnabled, VmMemoryMetrics, - VmRevertReason, VmTrackingContracts, + VmExecutionStatistics, VmFactory, VmInterface, VmInterfaceHistoryEnabled, VmRevertReason, + VmTrackingContracts, }, utils::events::extract_l2tol1logs_from_l1_messenger, vm_fast::{ @@ -670,10 +670,6 @@ impl VmInterface for Vm { self.bootloader_state.start_new_l2_block(l2_block_env) } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - todo!("Unused during batch execution") - } - fn finish_batch(&mut self) -> FinishedL1Batch { let result = self.inspect(&mut Tr::default(), VmExecutionMode::Batch); let execution_state = self.get_current_execution_state(); diff --git a/core/lib/multivm/src/versions/vm_latest/implementation/statistics.rs b/core/lib/multivm/src/versions/vm_latest/implementation/statistics.rs index 34c1e1f81da1..c1cf15043562 100644 --- a/core/lib/multivm/src/versions/vm_latest/implementation/statistics.rs +++ b/core/lib/multivm/src/versions/vm_latest/implementation/statistics.rs @@ -51,7 +51,7 @@ impl Vm { } /// Returns the info about all oracles' sizes. - pub(crate) fn record_vm_memory_metrics_inner(&self) -> VmMemoryMetrics { + pub(crate) fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { VmMemoryMetrics { event_sink_inner: self.state.event_sink.get_size(), event_sink_history: self.state.event_sink.get_history_size(), diff --git a/core/lib/multivm/src/versions/vm_latest/vm.rs b/core/lib/multivm/src/versions/vm_latest/vm.rs index 506b6666ecd9..8ccd600a79e1 100644 --- a/core/lib/multivm/src/versions/vm_latest/vm.rs +++ b/core/lib/multivm/src/versions/vm_latest/vm.rs @@ -13,7 +13,7 @@ use crate::{ BytecodeCompressionError, BytecodeCompressionResult, CurrentExecutionState, FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceHistoryEnabled, - VmMemoryMetrics, VmTrackingContracts, + VmTrackingContracts, }, utils::events::extract_l2tol1logs_from_l1_messenger, vm_latest::{ @@ -161,10 +161,6 @@ impl VmInterface for Vm { } } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - self.record_vm_memory_metrics_inner() - } - fn finish_batch(&mut self) -> FinishedL1Batch { let result = self.inspect(&mut TracerDispatcher::default(), VmExecutionMode::Batch); let execution_state = self.get_current_execution_state(); diff --git a/core/lib/multivm/src/versions/vm_m5/vm.rs b/core/lib/multivm/src/versions/vm_m5/vm.rs index 40f66659f29f..5a26506f3463 100644 --- a/core/lib/multivm/src/versions/vm_m5/vm.rs +++ b/core/lib/multivm/src/versions/vm_m5/vm.rs @@ -50,6 +50,10 @@ impl Vm { _phantom: Default::default(), } } + + pub(crate) fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { + VmMemoryMetrics::default() + } } impl VmInterface for Vm { @@ -106,19 +110,6 @@ impl VmInterface for Vm { ) } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - VmMemoryMetrics { - event_sink_inner: 0, - event_sink_history: 0, - memory_inner: 0, - memory_history: 0, - decommittment_processor_inner: 0, - decommittment_processor_history: 0, - storage_inner: 0, - storage_history: 0, - } - } - fn finish_batch(&mut self) -> FinishedL1Batch { self.vm .execute_till_block_end( diff --git a/core/lib/multivm/src/versions/vm_m6/vm.rs b/core/lib/multivm/src/versions/vm_m6/vm.rs index 627687a5524a..1fdc8ae64f80 100644 --- a/core/lib/multivm/src/versions/vm_m6/vm.rs +++ b/core/lib/multivm/src/versions/vm_m6/vm.rs @@ -50,6 +50,23 @@ impl Vm { system_env, } } + + pub(crate) fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { + VmMemoryMetrics { + event_sink_inner: self.vm.state.event_sink.get_size(), + event_sink_history: self.vm.state.event_sink.get_history_size(), + memory_inner: self.vm.state.memory.get_size(), + memory_history: self.vm.state.memory.get_history_size(), + decommittment_processor_inner: self.vm.state.decommittment_processor.get_size(), + decommittment_processor_history: self + .vm + .state + .decommittment_processor + .get_history_size(), + storage_inner: self.vm.state.storage.get_size(), + storage_history: self.vm.state.storage.get_history_size(), + } + } } impl VmInterface for Vm { @@ -186,23 +203,6 @@ impl VmInterface for Vm { } } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - VmMemoryMetrics { - event_sink_inner: self.vm.state.event_sink.get_size(), - event_sink_history: self.vm.state.event_sink.get_history_size(), - memory_inner: self.vm.state.memory.get_size(), - memory_history: self.vm.state.memory.get_history_size(), - decommittment_processor_inner: self.vm.state.decommittment_processor.get_size(), - decommittment_processor_history: self - .vm - .state - .decommittment_processor - .get_history_size(), - storage_inner: self.vm.state.storage.get_size(), - storage_history: self.vm.state.storage.get_history_size(), - } - } - fn finish_batch(&mut self) -> FinishedL1Batch { self.vm .execute_till_block_end( diff --git a/core/lib/multivm/src/versions/vm_refunds_enhancement/implementation/statistics.rs b/core/lib/multivm/src/versions/vm_refunds_enhancement/implementation/statistics.rs index dcda1457b765..a73c212db29c 100644 --- a/core/lib/multivm/src/versions/vm_refunds_enhancement/implementation/statistics.rs +++ b/core/lib/multivm/src/versions/vm_refunds_enhancement/implementation/statistics.rs @@ -56,7 +56,7 @@ impl Vm { } /// Returns the info about all oracles' sizes. - pub(crate) fn record_vm_memory_metrics_inner(&self) -> VmMemoryMetrics { + pub(crate) fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { VmMemoryMetrics { event_sink_inner: self.state.event_sink.get_size(), event_sink_history: self.state.event_sink.get_history_size(), diff --git a/core/lib/multivm/src/versions/vm_refunds_enhancement/vm.rs b/core/lib/multivm/src/versions/vm_refunds_enhancement/vm.rs index 735bd29c3b09..d87fd4d104da 100644 --- a/core/lib/multivm/src/versions/vm_refunds_enhancement/vm.rs +++ b/core/lib/multivm/src/versions/vm_refunds_enhancement/vm.rs @@ -8,7 +8,6 @@ use crate::{ BytecodeCompressionError, BytecodeCompressionResult, CurrentExecutionState, FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceHistoryEnabled, - VmMemoryMetrics, }, vm_latest::HistoryEnabled, vm_refunds_enhancement::{ @@ -118,10 +117,6 @@ impl VmInterface for Vm { } } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - self.record_vm_memory_metrics_inner() - } - fn finish_batch(&mut self) -> FinishedL1Batch { let result = self.inspect(&mut TracerDispatcher::default(), VmExecutionMode::Batch); let execution_state = self.get_current_execution_state(); diff --git a/core/lib/multivm/src/versions/vm_virtual_blocks/implementation/statistics.rs b/core/lib/multivm/src/versions/vm_virtual_blocks/implementation/statistics.rs index d082085a1550..dbd8813035e9 100644 --- a/core/lib/multivm/src/versions/vm_virtual_blocks/implementation/statistics.rs +++ b/core/lib/multivm/src/versions/vm_virtual_blocks/implementation/statistics.rs @@ -56,7 +56,7 @@ impl Vm { } /// Returns the info about all oracles' sizes. - pub(crate) fn record_vm_memory_metrics_inner(&self) -> VmMemoryMetrics { + pub(crate) fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { VmMemoryMetrics { event_sink_inner: self.state.event_sink.get_size(), event_sink_history: self.state.event_sink.get_history_size(), diff --git a/core/lib/multivm/src/versions/vm_virtual_blocks/vm.rs b/core/lib/multivm/src/versions/vm_virtual_blocks/vm.rs index 2a9d6eed6c7d..28c09590f2ad 100644 --- a/core/lib/multivm/src/versions/vm_virtual_blocks/vm.rs +++ b/core/lib/multivm/src/versions/vm_virtual_blocks/vm.rs @@ -8,7 +8,6 @@ use crate::{ BytecodeCompressionError, BytecodeCompressionResult, CurrentExecutionState, FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceHistoryEnabled, - VmMemoryMetrics, }, vm_latest::HistoryEnabled, vm_virtual_blocks::{ @@ -118,10 +117,6 @@ impl VmInterface for Vm { } } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - self.record_vm_memory_metrics_inner() - } - fn finish_batch(&mut self) -> FinishedL1Batch { let result = self.inspect(&mut TracerDispatcher::default(), VmExecutionMode::Batch); let execution_state = self.get_current_execution_state(); diff --git a/core/lib/multivm/src/vm_instance.rs b/core/lib/multivm/src/vm_instance.rs index 8dd67e1ac4e2..ac5693b61619 100644 --- a/core/lib/multivm/src/vm_instance.rs +++ b/core/lib/multivm/src/vm_instance.rs @@ -87,10 +87,6 @@ impl VmInterface for LegacyVmInstance { )) } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - dispatch_legacy_vm!(self.record_vm_memory_metrics()) - } - /// Return the results of execution of all batch fn finish_batch(&mut self) -> FinishedL1Batch { dispatch_legacy_vm!(self.finish_batch()) @@ -213,6 +209,11 @@ impl LegacyVmInstance { } } } + + /// Returns memory-related oracle metrics. + pub fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { + dispatch_legacy_vm!(self.record_vm_memory_metrics()) + } } /// Fast VM shadowed by the latest legacy VM. @@ -283,10 +284,6 @@ impl VmInterface for FastVmInsta } } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - dispatch_fast_vm!(self.record_vm_memory_metrics()) - } - fn finish_batch(&mut self) -> FinishedL1Batch { dispatch_fast_vm!(self.finish_batch()) } diff --git a/core/lib/vm_interface/src/types/outputs/statistic.rs b/core/lib/vm_interface/src/types/outputs/statistic.rs index 095547076d42..f8e3851c8321 100644 --- a/core/lib/vm_interface/src/types/outputs/statistic.rs +++ b/core/lib/vm_interface/src/types/outputs/statistic.rs @@ -109,7 +109,8 @@ pub struct VmExecutionStatistics { pub circuit_statistic: CircuitStatistic, } -/// Oracle metrics of the VM. +/// Oracle metrics reported by legacy VMs. +#[derive(Debug, Default)] pub struct VmMemoryMetrics { pub event_sink_inner: usize, pub event_sink_history: usize, diff --git a/core/lib/vm_interface/src/utils/dump.rs b/core/lib/vm_interface/src/utils/dump.rs index 5dc2351dcf7a..288c6445494d 100644 --- a/core/lib/vm_interface/src/utils/dump.rs +++ b/core/lib/vm_interface/src/utils/dump.rs @@ -7,7 +7,7 @@ use crate::{ storage::{ReadStorage, StoragePtr, StorageSnapshot, StorageView}, BytecodeCompressionResult, FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, VmInterfaceExt, VmInterfaceHistoryEnabled, - VmMemoryMetrics, VmTrackingContracts, + VmTrackingContracts, }; fn create_storage_snapshot( @@ -177,10 +177,6 @@ impl VmInterface for DumpingVm { .inspect_transaction_with_bytecode_compression(tracer, tx, with_compression) } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - self.inner.record_vm_memory_metrics() - } - fn finish_batch(&mut self) -> FinishedL1Batch { self.inner.finish_batch() } diff --git a/core/lib/vm_interface/src/utils/shadow.rs b/core/lib/vm_interface/src/utils/shadow.rs index d0b3697d5ea5..92eb65a810f7 100644 --- a/core/lib/vm_interface/src/utils/shadow.rs +++ b/core/lib/vm_interface/src/utils/shadow.rs @@ -12,7 +12,7 @@ use crate::{ storage::{ReadStorage, StoragePtr, StorageView}, BytecodeCompressionResult, CurrentExecutionState, FinishedL1Batch, L1BatchEnv, L2BlockEnv, SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmFactory, VmInterface, - VmInterfaceHistoryEnabled, VmMemoryMetrics, VmTrackingContracts, + VmInterfaceHistoryEnabled, VmTrackingContracts, }; /// Handler for VM divergences. @@ -233,10 +233,6 @@ where (main_bytecodes_result, main_tx_result) } - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics { - self.main.record_vm_memory_metrics() - } - fn finish_batch(&mut self) -> FinishedL1Batch { let main_batch = self.main.finish_batch(); if let Some(shadow) = self.shadow.get_mut() { diff --git a/core/lib/vm_interface/src/vm.rs b/core/lib/vm_interface/src/vm.rs index 90ae76be8057..37e33a92b509 100644 --- a/core/lib/vm_interface/src/vm.rs +++ b/core/lib/vm_interface/src/vm.rs @@ -15,7 +15,7 @@ use zksync_types::{Transaction, H256}; use crate::{ storage::StoragePtr, BytecodeCompressionResult, FinishedL1Batch, L1BatchEnv, L2BlockEnv, - SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, VmMemoryMetrics, + SystemEnv, VmExecutionMode, VmExecutionResultAndLogs, }; pub trait VmInterface { @@ -44,9 +44,6 @@ pub trait VmInterface { with_compression: bool, ) -> (BytecodeCompressionResult<'_>, VmExecutionResultAndLogs); - /// Record VM memory metrics. - fn record_vm_memory_metrics(&self) -> VmMemoryMetrics; - /// Execute batch till the end and return the result, with final execution state /// and bootloader memory. fn finish_batch(&mut self) -> FinishedL1Batch; From d3a104765112bdc577ae4ee337d37f18e9380207 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 7 Oct 2024 12:43:51 +0300 Subject: [PATCH 11/11] Fix misc issues --- .../src/versions/vm_fast/tests/rollbacks.rs | 1 - core/lib/multivm/src/versions/vm_fast/vm.rs | 37 +++++++++---------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/core/lib/multivm/src/versions/vm_fast/tests/rollbacks.rs b/core/lib/multivm/src/versions/vm_fast/tests/rollbacks.rs index 6d29e97456f9..cff72d8ec5aa 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/rollbacks.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/rollbacks.rs @@ -172,6 +172,5 @@ fn rollback_in_call_mode() { ExecutionResult::Revert { output } if output.to_string().contains("This method always reverts") ); - // The new VM performs deduplication internally and doesn't record reads, hence this crude comparison. assert_eq!(vm_result.logs.storage_logs, []); } diff --git a/core/lib/multivm/src/versions/vm_fast/vm.rs b/core/lib/multivm/src/versions/vm_fast/vm.rs index c6b3f4411e60..10be6d88b045 100644 --- a/core/lib/multivm/src/versions/vm_fast/vm.rs +++ b/core/lib/multivm/src/versions/vm_fast/vm.rs @@ -175,31 +175,27 @@ impl Vm { let mut last_tx_result = None; let mut pubdata_before = self.inner.pubdata() as u32; let mut pubdata_published = 0; - let mut execution_ended = false; - let execution_result = loop { + let (execution_result, execution_ended) = loop { let hook = match self.inner.run(&mut self.world, tracer) { ExecutionEnd::SuspendedOnHook(hook) => hook, ExecutionEnd::ProgramFinished(output) => { - execution_ended = true; - break ExecutionResult::Success { output }; + break (ExecutionResult::Success { output }, true); } ExecutionEnd::Reverted(output) => { - execution_ended = true; - break match TxRevertReason::parse_error(&output) { + let result = match TxRevertReason::parse_error(&output) { TxRevertReason::TxReverted(output) => ExecutionResult::Revert { output }, TxRevertReason::Halt(reason) => ExecutionResult::Halt { reason }, }; + break (result, true); } ExecutionEnd::Panicked => { - execution_ended = true; - break ExecutionResult::Halt { - reason: if self.gas_remaining() == 0 { - Halt::BootloaderOutOfGas - } else { - Halt::VMPanic - }, + let reason = if self.gas_remaining() == 0 { + Halt::BootloaderOutOfGas + } else { + Halt::VMPanic }; + break (ExecutionResult::Halt { reason }, true); } }; @@ -209,7 +205,7 @@ impl Vm { } Hook::TxHasEnded => { if let VmExecutionMode::OneTx = execution_mode { - break last_tx_result.take().unwrap(); + break (last_tx_result.take().unwrap(), false); } } Hook::AskOperatorForRefund => { @@ -785,6 +781,10 @@ impl zksync_vm2::StorageInterface for World { fn read_storage(&mut self, contract: H160, key: U256) -> StorageSlot { let key = &StorageKey::new(AccountTreeId::new(contract), u256_to_h256(key)); let value = U256::from_big_endian(self.storage.read_value(key).as_bytes()); + // `is_write_initial` value can be true even if the slot has previously been written to / has non-zero value! + // This can happen during oneshot execution (i.e., executing a single transaction) since it emulates + // execution starting in the middle of a batch in the general case. Hence, a slot that was first written to in the batch + // must still be considered an initial write by the refund logic. let is_write_initial = self.storage.is_write_initial(key); StorageSlot { value, @@ -797,8 +797,8 @@ impl zksync_vm2::StorageInterface for World { U256::from_big_endian(self.storage.read_value(key).as_bytes()) } - fn cost_of_writing_storage(&mut self, initial_slot: StorageSlot, new_value: U256) -> u32 { - if initial_slot.value == new_value { + fn cost_of_writing_storage(&mut self, slot: StorageSlot, new_value: U256) -> u32 { + if slot.value == new_value { return 0; } @@ -812,10 +812,9 @@ impl zksync_vm2::StorageInterface for World { // For value compression, we use a metadata byte which holds the length of the value and the operation from the // previous state to the new state, and the compressed value. The maximum for this is 33 bytes. // Total bytes for initial writes then becomes 65 bytes and repeated writes becomes 38 bytes. - let compressed_value_size = - compress_with_best_strategy(initial_slot.value, new_value).len() as u32; + let compressed_value_size = compress_with_best_strategy(slot.value, new_value).len() as u32; - if initial_slot.is_write_initial { + if slot.is_write_initial { (BYTES_PER_DERIVED_KEY as u32) + compressed_value_size } else { (BYTES_PER_ENUMERATION_INDEX as u32) + compressed_value_size