From 84986f44b32d0a7fa68c3f60b1ec266ce663e778 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Wed, 23 Oct 2024 17:20:20 +0300 Subject: [PATCH] test(vm): Run `multivm` tests with shadowing (#3137) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ Runs shared unit tests in the `multivm` crate for the shadowed VM. ## Why ❔ Allows to cheaply check VM divergences. ## Checklist - [x] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [x] Tests for the changes have been added / updated. - [x] Documentation comments have been added / updated. - [x] Code has been formatted via `zkstack dev fmt` and `zkstack dev lint`. --- core/lib/multivm/README.md | 2 + core/lib/multivm/src/versions/mod.rs | 2 + .../{testonly/shadow.rs => shadow/mod.rs} | 2 + core/lib/multivm/src/versions/shadow/tests.rs | 415 ++++++++++++++++++ .../versions/testonly/get_used_contracts.rs | 25 +- core/lib/multivm/src/versions/testonly/mod.rs | 12 +- .../src/versions/testonly/tester/mod.rs | 2 +- .../src/versions/vm_latest/tests/mod.rs | 9 +- core/lib/vm_interface/src/utils/dump.rs | 12 + core/lib/vm_interface/src/utils/mod.rs | 4 +- core/lib/vm_interface/src/utils/shadow.rs | 340 ++++++++------ 11 files changed, 674 insertions(+), 151 deletions(-) rename core/lib/multivm/src/versions/{testonly/shadow.rs => shadow/mod.rs} (99%) create mode 100644 core/lib/multivm/src/versions/shadow/tests.rs diff --git a/core/lib/multivm/README.md b/core/lib/multivm/README.md index f5e8a552242..34883db5990 100644 --- a/core/lib/multivm/README.md +++ b/core/lib/multivm/README.md @@ -14,5 +14,7 @@ If you want to add unit tests for the VM wrapper, consider the following: - Whenever possible, make tests reusable; declare test logic in the [`testonly`](src/versions/testonly/mod.rs) module, and then instantiate tests using this logic for the supported VM versions. If necessary, extend the tested VM trait so that test logic can be defined in a generic way. See the `testonly` module docs for more detailed guidelines. +- If you define a generic test, don't forget to add its instantiations for all supported VMs (`vm_latest`, `vm_fast` and + `shadow`). `shadow` tests allow checking VM divergences for free! - Do not use an RNG where it can be avoided (e.g., for test contract addresses). - Avoid using zero / default values in cases they can be treated specially by the tested code. diff --git a/core/lib/multivm/src/versions/mod.rs b/core/lib/multivm/src/versions/mod.rs index 1df706a6cce..b6523b3d474 100644 --- a/core/lib/multivm/src/versions/mod.rs +++ b/core/lib/multivm/src/versions/mod.rs @@ -1,3 +1,5 @@ +#[cfg(test)] +mod shadow; mod shared; #[cfg(test)] mod testonly; diff --git a/core/lib/multivm/src/versions/testonly/shadow.rs b/core/lib/multivm/src/versions/shadow/mod.rs similarity index 99% rename from core/lib/multivm/src/versions/testonly/shadow.rs rename to core/lib/multivm/src/versions/shadow/mod.rs index 6a7d42b06fc..fe9ce8eefcb 100644 --- a/core/lib/multivm/src/versions/testonly/shadow.rs +++ b/core/lib/multivm/src/versions/shadow/mod.rs @@ -28,6 +28,8 @@ use crate::{ vm_latest::HistoryEnabled, }; +mod tests; + type ReferenceVm = vm_latest::Vm, HistoryEnabled>; type ShadowedFastVm = crate::vm_instance::ShadowedFastVm; diff --git a/core/lib/multivm/src/versions/shadow/tests.rs b/core/lib/multivm/src/versions/shadow/tests.rs new file mode 100644 index 00000000000..64179f59be1 --- /dev/null +++ b/core/lib/multivm/src/versions/shadow/tests.rs @@ -0,0 +1,415 @@ +//! Unit tests from the `testonly` test suite. + +use std::collections::HashSet; + +use zksync_types::{writes::StateDiffRecord, StorageKey, Transaction, H256, U256}; + +use super::ShadowedFastVm; +use crate::{ + interface::{ + utils::{ShadowMut, ShadowRef}, + CurrentExecutionState, L2BlockEnv, VmExecutionMode, VmExecutionResultAndLogs, + }, + versions::testonly::TestedVm, +}; + +impl TestedVm for ShadowedFastVm { + type StateDump = (); + + fn dump_state(&self) -> Self::StateDump { + // Do nothing + } + + fn gas_remaining(&mut self) -> u32 { + self.get_mut("gas_remaining", |r| match r { + ShadowMut::Main(vm) => vm.gas_remaining(), + ShadowMut::Shadow(vm) => vm.gas_remaining(), + }) + } + + fn get_current_execution_state(&self) -> CurrentExecutionState { + self.get_custom("current_execution_state", |r| match r { + ShadowRef::Main(vm) => vm.get_current_execution_state(), + ShadowRef::Shadow(vm) => vm.get_current_execution_state(), + }) + } + + fn decommitted_hashes(&self) -> HashSet { + self.get("decommitted_hashes", |r| match r { + ShadowRef::Main(vm) => vm.decommitted_hashes(), + ShadowRef::Shadow(vm) => TestedVm::decommitted_hashes(vm), + }) + } + + fn execute_with_state_diffs( + &mut self, + diffs: Vec, + mode: VmExecutionMode, + ) -> VmExecutionResultAndLogs { + self.get_custom_mut("execute_with_state_diffs", |r| match r { + ShadowMut::Main(vm) => vm.execute_with_state_diffs(diffs.clone(), mode), + ShadowMut::Shadow(vm) => vm.execute_with_state_diffs(diffs.clone(), mode), + }) + } + + fn insert_bytecodes(&mut self, bytecodes: &[&[u8]]) { + self.get_mut("insert_bytecodes", |r| match r { + ShadowMut::Main(vm) => vm.insert_bytecodes(bytecodes), + ShadowMut::Shadow(vm) => TestedVm::insert_bytecodes(vm, bytecodes), + }); + } + + fn known_bytecode_hashes(&self) -> HashSet { + self.get("known_bytecode_hashes", |r| match r { + ShadowRef::Main(vm) => vm.known_bytecode_hashes(), + ShadowRef::Shadow(vm) => vm.known_bytecode_hashes(), + }) + } + + fn manually_decommit(&mut self, code_hash: H256) -> bool { + self.get_mut("manually_decommit", |r| match r { + ShadowMut::Main(vm) => vm.manually_decommit(code_hash), + ShadowMut::Shadow(vm) => vm.manually_decommit(code_hash), + }) + } + + fn verify_required_bootloader_heap(&self, cells: &[(u32, U256)]) { + self.get("verify_required_bootloader_heap", |r| match r { + ShadowRef::Main(vm) => vm.verify_required_bootloader_heap(cells), + ShadowRef::Shadow(vm) => vm.verify_required_bootloader_heap(cells), + }); + } + + fn write_to_bootloader_heap(&mut self, cells: &[(usize, U256)]) { + self.get_mut("manually_decommit", |r| match r { + ShadowMut::Main(vm) => vm.write_to_bootloader_heap(cells), + ShadowMut::Shadow(vm) => TestedVm::write_to_bootloader_heap(vm, cells), + }); + } + + fn read_storage(&mut self, key: StorageKey) -> U256 { + self.get_mut("read_storage", |r| match r { + ShadowMut::Main(vm) => vm.read_storage(key), + ShadowMut::Shadow(vm) => vm.read_storage(key), + }) + } + + fn last_l2_block_hash(&self) -> H256 { + self.get("last_l2_block_hash", |r| match r { + ShadowRef::Main(vm) => vm.last_l2_block_hash(), + ShadowRef::Shadow(vm) => vm.last_l2_block_hash(), + }) + } + + fn push_l2_block_unchecked(&mut self, block: L2BlockEnv) { + self.get_mut("push_l2_block_unchecked", |r| match r { + ShadowMut::Main(vm) => vm.push_l2_block_unchecked(block), + ShadowMut::Shadow(vm) => vm.push_l2_block_unchecked(block), + }); + } + + fn push_transaction_with_refund(&mut self, tx: Transaction, refund: u64) { + self.get_mut("push_transaction_with_refund", |r| match r { + ShadowMut::Main(vm) => vm.push_transaction_with_refund(tx.clone(), refund), + ShadowMut::Shadow(vm) => vm.push_transaction_with_refund(tx.clone(), refund), + }); + } +} + +mod block_tip { + use crate::versions::testonly::block_tip::*; + + #[test] + fn dry_run_upper_bound() { + test_dry_run_upper_bound::(); + } +} + +mod bootloader { + use crate::versions::testonly::bootloader::*; + + #[test] + fn dummy_bootloader() { + test_dummy_bootloader::(); + } + + #[test] + fn bootloader_out_of_gas() { + test_bootloader_out_of_gas::(); + } +} + +mod bytecode_publishing { + use crate::versions::testonly::bytecode_publishing::*; + + #[test] + fn bytecode_publishing() { + test_bytecode_publishing::(); + } +} + +mod circuits { + use crate::versions::testonly::circuits::*; + + #[test] + fn circuits() { + test_circuits::(); + } +} + +mod code_oracle { + use crate::versions::testonly::code_oracle::*; + + #[test] + fn code_oracle() { + test_code_oracle::(); + } + + #[test] + fn code_oracle_big_bytecode() { + test_code_oracle_big_bytecode::(); + } + + #[test] + fn refunds_in_code_oracle() { + test_refunds_in_code_oracle::(); + } +} + +mod default_aa { + use crate::versions::testonly::default_aa::*; + + #[test] + fn default_aa_interaction() { + test_default_aa_interaction::(); + } +} + +mod gas_limit { + use crate::versions::testonly::gas_limit::*; + + #[test] + fn tx_gas_limit_offset() { + test_tx_gas_limit_offset::(); + } +} + +mod get_used_contracts { + use crate::versions::testonly::get_used_contracts::*; + + #[test] + fn get_used_contracts() { + test_get_used_contracts::(); + } + + #[test] + fn get_used_contracts_with_far_call() { + test_get_used_contracts_with_far_call::(); + } + + #[test] + fn get_used_contracts_with_out_of_gas_far_call() { + test_get_used_contracts_with_out_of_gas_far_call::(); + } +} + +mod is_write_initial { + use crate::versions::testonly::is_write_initial::*; + + #[test] + fn is_write_initial_behaviour() { + test_is_write_initial_behaviour::(); + } +} + +mod l1_tx_execution { + use crate::versions::testonly::l1_tx_execution::*; + + #[test] + fn l1_tx_execution() { + test_l1_tx_execution::(); + } + + #[test] + fn l1_tx_execution_high_gas_limit() { + test_l1_tx_execution_high_gas_limit::(); + } +} + +mod l2_blocks { + use crate::versions::testonly::l2_blocks::*; + + #[test] + fn l2_block_initialization_timestamp() { + test_l2_block_initialization_timestamp::(); + } + + #[test] + fn l2_block_initialization_number_non_zero() { + test_l2_block_initialization_number_non_zero::(); + } + + #[test] + fn l2_block_same_l2_block() { + test_l2_block_same_l2_block::(); + } + + #[test] + fn l2_block_new_l2_block() { + test_l2_block_new_l2_block::(); + } + + #[test] + fn l2_block_first_in_batch() { + test_l2_block_first_in_batch::(); + } +} + +mod nonce_holder { + use crate::versions::testonly::nonce_holder::*; + + #[test] + fn nonce_holder() { + test_nonce_holder::(); + } +} + +mod precompiles { + use crate::versions::testonly::precompiles::*; + + #[test] + fn keccak() { + test_keccak::(); + } + + #[test] + fn sha256() { + test_sha256::(); + } + + #[test] + fn ecrecover() { + test_ecrecover::(); + } +} + +mod refunds { + use crate::versions::testonly::refunds::*; + + #[test] + fn predetermined_refunded_gas() { + test_predetermined_refunded_gas::(); + } + + #[test] + fn negative_pubdata_for_transaction() { + test_negative_pubdata_for_transaction::(); + } +} + +mod require_eip712 { + use crate::versions::testonly::require_eip712::*; + + #[test] + fn require_eip712() { + test_require_eip712::(); + } +} + +mod rollbacks { + use crate::versions::testonly::rollbacks::*; + + #[test] + fn vm_rollbacks() { + test_vm_rollbacks::(); + } + + #[test] + fn vm_loadnext_rollbacks() { + test_vm_loadnext_rollbacks::(); + } + + #[test] + fn rollback_in_call_mode() { + test_rollback_in_call_mode::(); + } +} + +mod secp256r1 { + use crate::versions::testonly::secp256r1::*; + + #[test] + fn secp256r1() { + test_secp256r1::(); + } +} + +mod simple_execution { + use crate::versions::testonly::simple_execution::*; + + #[test] + fn estimate_fee() { + test_estimate_fee::(); + } + + #[test] + fn simple_execute() { + test_simple_execute::(); + } +} + +mod storage { + use crate::versions::testonly::storage::*; + + #[test] + fn storage_behavior() { + test_storage_behavior::(); + } + + #[test] + fn transient_storage_behavior() { + test_transient_storage_behavior::(); + } +} + +mod tracing_execution_error { + use crate::versions::testonly::tracing_execution_error::*; + + #[test] + fn tracing_of_execution_errors() { + test_tracing_of_execution_errors::(); + } +} + +mod transfer { + use crate::versions::testonly::transfer::*; + + #[test] + fn send_and_transfer() { + test_send_and_transfer::(); + } + + #[test] + fn reentrancy_protection_send_and_transfer() { + test_reentrancy_protection_send_and_transfer::(); + } +} + +mod upgrade { + use crate::versions::testonly::upgrade::*; + + #[test] + fn protocol_upgrade_is_first() { + test_protocol_upgrade_is_first::(); + } + + #[test] + fn force_deploy_upgrade() { + test_force_deploy_upgrade::(); + } + + #[test] + fn complex_upgrader() { + test_complex_upgrader::(); + } +} diff --git a/core/lib/multivm/src/versions/testonly/get_used_contracts.rs b/core/lib/multivm/src/versions/testonly/get_used_contracts.rs index fbad94a0eee..d3ffee20c34 100644 --- a/core/lib/multivm/src/versions/testonly/get_used_contracts.rs +++ b/core/lib/multivm/src/versions/testonly/get_used_contracts.rs @@ -1,4 +1,4 @@ -use std::{collections::HashSet, iter}; +use std::iter; use assert_matches::assert_matches; use ethabi::Token; @@ -11,7 +11,7 @@ use zksync_utils::{bytecode::hash_bytecode, h256_to_u256}; use super::{ read_proxy_counter_contract, read_test_contract, tester::{VmTester, VmTesterBuilder}, - TestedVm, BASE_SYSTEM_CONTRACTS, + TestedVm, }; use crate::{ interface::{ @@ -27,7 +27,7 @@ pub(crate) fn test_get_used_contracts() { .with_rich_accounts(1) .build::(); - assert!(known_bytecodes_without_base_system_contracts(&vm.vm).is_empty()); + assert!(vm.vm.known_bytecode_hashes().is_empty()); // create and push and execute some not-empty factory deps transaction with success status // to check that `get_decommitted_hashes()` updates @@ -44,10 +44,7 @@ pub(crate) fn test_get_used_contracts() { .contains(&h256_to_u256(tx.bytecode_hash))); // Note: `Default_AA` will be in the list of used contracts if L2 tx is used - assert_eq!( - vm.vm.decommitted_hashes(), - known_bytecodes_without_base_system_contracts(&vm.vm) - ); + assert_eq!(vm.vm.decommitted_hashes(), vm.vm.known_bytecode_hashes()); // create push and execute some non-empty factory deps transaction that fails // (`known_bytecodes` will be updated but we expect `get_decommitted_hashes()` to not be updated) @@ -80,23 +77,11 @@ pub(crate) fn test_get_used_contracts() { for factory_dep in tx2.execute.factory_deps { let hash = hash_bytecode(&factory_dep); let hash_to_u256 = h256_to_u256(hash); - assert!(known_bytecodes_without_base_system_contracts(&vm.vm).contains(&hash_to_u256)); + assert!(vm.vm.known_bytecode_hashes().contains(&hash_to_u256)); assert!(!vm.vm.decommitted_hashes().contains(&hash_to_u256)); } } -fn known_bytecodes_without_base_system_contracts(vm: &impl TestedVm) -> HashSet { - let mut known_bytecodes_without_base_system_contracts = vm.known_bytecode_hashes(); - known_bytecodes_without_base_system_contracts - .remove(&h256_to_u256(BASE_SYSTEM_CONTRACTS.default_aa.hash)); - if let Some(evm_emulator) = &BASE_SYSTEM_CONTRACTS.evm_emulator { - let was_removed = - known_bytecodes_without_base_system_contracts.remove(&h256_to_u256(evm_emulator.hash)); - assert!(was_removed); - } - known_bytecodes_without_base_system_contracts -} - /// Counter test contract bytecode inflated by appending lots of `NOP` opcodes at the end. This leads to non-trivial /// decommitment cost (>10,000 gas). fn inflated_counter_bytecode() -> Vec { diff --git a/core/lib/multivm/src/versions/testonly/mod.rs b/core/lib/multivm/src/versions/testonly/mod.rs index 838ba98a9aa..74cda6a9522 100644 --- a/core/lib/multivm/src/versions/testonly/mod.rs +++ b/core/lib/multivm/src/versions/testonly/mod.rs @@ -9,6 +9,8 @@ //! - Tests use [`VmTester`] built using [`VmTesterBuilder`] to create a VM instance. This allows to set up storage for the VM, //! custom [`SystemEnv`] / [`L1BatchEnv`], deployed contracts, pre-funded accounts etc. +use std::collections::HashSet; + use ethabi::Contract; use once_cell::sync::Lazy; use zksync_contracts::{ @@ -20,7 +22,7 @@ use zksync_types::{ utils::storage_key_for_eth_balance, Address, L1BatchNumber, L2BlockNumber, L2ChainId, ProtocolVersionId, U256, }; -use zksync_utils::{bytecode::hash_bytecode, bytes_to_be_words, u256_to_h256}; +use zksync_utils::{bytecode::hash_bytecode, bytes_to_be_words, h256_to_u256, u256_to_h256}; use zksync_vm_interface::{L1BatchEnv, L2BlockEnv, SystemEnv, TxExecutionMode}; pub(super) use self::tester::{TestedVm, VmTester, VmTesterBuilder}; @@ -45,7 +47,6 @@ pub(super) mod refunds; pub(super) mod require_eip712; pub(super) mod rollbacks; pub(super) mod secp256r1; -mod shadow; pub(super) mod simple_execution; pub(super) mod storage; mod tester; @@ -133,6 +134,13 @@ pub(crate) fn get_bootloader(test: &str) -> SystemContractCode { } } +pub(crate) fn filter_out_base_system_contracts(all_bytecode_hashes: &mut HashSet) { + all_bytecode_hashes.remove(&h256_to_u256(BASE_SYSTEM_CONTRACTS.default_aa.hash)); + if let Some(evm_emulator) = &BASE_SYSTEM_CONTRACTS.evm_emulator { + all_bytecode_hashes.remove(&h256_to_u256(evm_emulator.hash)); + } +} + pub(super) fn default_system_env() -> SystemEnv { SystemEnv { zk_porter_available: false, diff --git a/core/lib/multivm/src/versions/testonly/tester/mod.rs b/core/lib/multivm/src/versions/testonly/tester/mod.rs index 4bab9bca610..7432322e0c8 100644 --- a/core/lib/multivm/src/versions/testonly/tester/mod.rs +++ b/core/lib/multivm/src/versions/testonly/tester/mod.rs @@ -195,7 +195,7 @@ pub(crate) trait TestedVm: fn insert_bytecodes(&mut self, bytecodes: &[&[u8]]); - /// Includes bytecodes that have failed to decommit. + /// Includes bytecodes that have failed to decommit. Should exclude base system contract bytecodes (default AA / EVM emulator). fn known_bytecode_hashes(&self) -> HashSet; /// Returns `true` iff the decommit is fresh. diff --git a/core/lib/multivm/src/versions/vm_latest/tests/mod.rs b/core/lib/multivm/src/versions/vm_latest/tests/mod.rs index 2835f5b6faa..6f748d543d3 100644 --- a/core/lib/multivm/src/versions/vm_latest/tests/mod.rs +++ b/core/lib/multivm/src/versions/vm_latest/tests/mod.rs @@ -14,7 +14,7 @@ use crate::{ storage::{InMemoryStorage, ReadStorage, StorageView, WriteStorage}, CurrentExecutionState, L2BlockEnv, VmExecutionMode, VmExecutionResultAndLogs, }, - versions::testonly::TestedVm, + versions::testonly::{filter_out_base_system_contracts, TestedVm}, vm_latest::{ constants::BOOTLOADER_HEAP_PAGE, old_vm::{event_sink::InMemoryEventSink, history_recorder::HistoryRecorder}, @@ -104,13 +104,16 @@ impl TestedVm for TestedLatestVm { } fn known_bytecode_hashes(&self) -> HashSet { - self.state + let mut bytecode_hashes: HashSet<_> = self + .state .decommittment_processor .known_bytecodes .inner() .keys() .copied() - .collect() + .collect(); + filter_out_base_system_contracts(&mut bytecode_hashes); + bytecode_hashes } fn manually_decommit(&mut self, code_hash: H256) -> bool { diff --git a/core/lib/vm_interface/src/utils/dump.rs b/core/lib/vm_interface/src/utils/dump.rs index 522a455a11b..4076aa72270 100644 --- a/core/lib/vm_interface/src/utils/dump.rs +++ b/core/lib/vm_interface/src/utils/dump.rs @@ -139,6 +139,18 @@ impl DumpingVm { } } +impl AsRef for DumpingVm { + fn as_ref(&self) -> &Vm { + &self.inner + } +} + +impl AsMut for DumpingVm { + fn as_mut(&mut self) -> &mut Vm { + &mut self.inner + } +} + impl VmInterface for DumpingVm { type TracerDispatcher = Vm::TracerDispatcher; diff --git a/core/lib/vm_interface/src/utils/mod.rs b/core/lib/vm_interface/src/utils/mod.rs index 80a51c7b144..394df7fc9a1 100644 --- a/core/lib/vm_interface/src/utils/mod.rs +++ b/core/lib/vm_interface/src/utils/mod.rs @@ -2,7 +2,9 @@ pub use self::{ dump::VmDump, - shadow::{DivergenceErrors, DivergenceHandler, ShadowVm}, + shadow::{ + CheckDivergence, DivergenceErrors, DivergenceHandler, ShadowMut, ShadowRef, ShadowVm, + }, }; mod dump; diff --git a/core/lib/vm_interface/src/utils/shadow.rs b/core/lib/vm_interface/src/utils/shadow.rs index 8cdc899238e..e8ef87c3c7f 100644 --- a/core/lib/vm_interface/src/utils/shadow.rs +++ b/core/lib/vm_interface/src/utils/shadow.rs @@ -1,4 +1,5 @@ use std::{ + any, cell::RefCell, collections::{BTreeMap, BTreeSet}, fmt, @@ -65,6 +66,154 @@ impl VmWithReporting { } } +/// Reference to either the main or shadow VM. +#[derive(Debug)] +pub enum ShadowRef<'a, Main, Shadow> { + /// Reference to the main VM. + Main(&'a Main), + /// Reference to the shadow VM. + Shadow(&'a Shadow), +} + +/// Mutable reference to either the main or shadow VM. +#[derive(Debug)] +pub enum ShadowMut<'a, Main, Shadow> { + /// Reference to the main VM. + Main(&'a mut Main), + /// Reference to the shadow VM. + Shadow(&'a mut Shadow), +} + +/// Type that can check divergence between its instances. +pub trait CheckDivergence { + /// Checks divergences and returns a list of divergence errors, if any. + fn check_divergence(&self, other: &Self) -> DivergenceErrors; +} + +#[derive(Debug)] +struct DivergingEq(T); + +impl CheckDivergence for DivergingEq { + fn check_divergence(&self, other: &Self) -> DivergenceErrors { + let mut errors = DivergenceErrors::new(); + errors.check_match(any::type_name::(), &self.0, &other.0); + errors + } +} + +impl CheckDivergence for CurrentExecutionState { + fn check_divergence(&self, other: &Self) -> DivergenceErrors { + let mut errors = DivergenceErrors::new(); + errors.check_match("final_state.events", &self.events, &other.events); + errors.check_match( + "final_state.user_l2_to_l1_logs", + &self.user_l2_to_l1_logs, + &other.user_l2_to_l1_logs, + ); + errors.check_match( + "final_state.system_logs", + &self.system_logs, + &other.system_logs, + ); + errors.check_match( + "final_state.storage_refunds", + &self.storage_refunds, + &other.storage_refunds, + ); + errors.check_match( + "final_state.pubdata_costs", + &self.pubdata_costs, + &other.pubdata_costs, + ); + errors.check_match( + "final_state.used_contract_hashes", + &self.used_contract_hashes.iter().collect::>(), + &other.used_contract_hashes.iter().collect::>(), + ); + + let main_deduplicated_logs = DivergenceErrors::gather_logs(&self.deduplicated_storage_logs); + let shadow_deduplicated_logs = + DivergenceErrors::gather_logs(&other.deduplicated_storage_logs); + errors.check_match( + "deduplicated_storage_logs", + &main_deduplicated_logs, + &shadow_deduplicated_logs, + ); + errors + } +} + +impl CheckDivergence for VmExecutionResultAndLogs { + fn check_divergence(&self, other: &Self) -> DivergenceErrors { + let mut errors = DivergenceErrors::new(); + errors.check_match("result", &self.result, &other.result); + errors.check_match("logs.events", &self.logs.events, &other.logs.events); + errors.check_match( + "logs.system_l2_to_l1_logs", + &self.logs.system_l2_to_l1_logs, + &other.logs.system_l2_to_l1_logs, + ); + errors.check_match( + "logs.user_l2_to_l1_logs", + &self.logs.user_l2_to_l1_logs, + &other.logs.user_l2_to_l1_logs, + ); + let main_logs = UniqueStorageLogs::new(&self.logs.storage_logs); + let shadow_logs = UniqueStorageLogs::new(&other.logs.storage_logs); + errors.check_match("logs.storage_logs", &main_logs, &shadow_logs); + errors.check_match("refunds", &self.refunds, &other.refunds); + errors.check_match( + "statistics.circuit_statistic", + &self.statistics.circuit_statistic, + &other.statistics.circuit_statistic, + ); + errors.check_match( + "statistics.pubdata_published", + &self.statistics.pubdata_published, + &other.statistics.pubdata_published, + ); + errors.check_match( + "statistics.gas_remaining", + &self.statistics.gas_remaining, + &other.statistics.gas_remaining, + ); + errors.check_match( + "statistics.gas_used", + &self.statistics.gas_used, + &other.statistics.gas_used, + ); + errors.check_match( + "statistics.computational_gas_used", + &self.statistics.computational_gas_used, + &other.statistics.computational_gas_used, + ); + errors + } +} + +impl CheckDivergence for FinishedL1Batch { + fn check_divergence(&self, other: &Self) -> DivergenceErrors { + let mut errors = DivergenceErrors::new(); + errors.extend( + self.block_tip_execution_result + .check_divergence(&other.block_tip_execution_result), + ); + errors.extend( + self.final_execution_state + .check_divergence(&other.final_execution_state), + ); + + errors.check_match( + "final_bootloader_memory", + &self.final_bootloader_memory, + &other.final_bootloader_memory, + ); + errors.check_match("pubdata_input", &self.pubdata_input, &other.pubdata_input); + errors.check_match("state_diffs", &self.state_diffs, &other.state_diffs); + errors + } +} + /// Shadowed VM that executes 2 VMs for each operation and compares their outputs. /// /// If a divergence is detected, the VM state is dumped using [a pluggable handler](Self::set_dump_handler()), @@ -105,6 +254,66 @@ where pub fn dump_state(&self) -> VmDump { self.main.dump_state() } + + /// Gets the specified value from both the main and shadow VM, checking whether it matches on both. + pub fn get(&self, name: &str, mut action: impl FnMut(ShadowRef<'_, Main, Shadow>) -> R) -> R + where + R: PartialEq + fmt::Debug + 'static, + { + self.get_custom(name, |r| DivergingEq(action(r))).0 + } + + /// Same as [`Self::get()`], but uses custom divergence checks for the type encapsulated in the [`CheckDivergence`] trait. + pub fn get_custom( + &self, + name: &str, + mut action: impl FnMut(ShadowRef<'_, Main, Shadow>) -> R, + ) -> R { + let main_output = action(ShadowRef::Main(self.main.as_ref())); + let borrow = self.shadow.borrow(); + if let Some(shadow) = &*borrow { + let shadow_output = action(ShadowRef::Shadow(&shadow.vm)); + let errors = main_output.check_divergence(&shadow_output); + if let Err(err) = errors.into_result() { + drop(borrow); + self.report_shared(err.context(format!("get({name})"))); + } + } + main_output + } + + /// Gets the specified value from both the main and shadow VM, potentially changing their state + /// and checking whether the returned value matches. + pub fn get_mut( + &mut self, + name: &str, + mut action: impl FnMut(ShadowMut<'_, Main, Shadow>) -> R, + ) -> R + where + R: PartialEq + fmt::Debug + 'static, + { + self.get_custom_mut(name, |r| DivergingEq(action(r))).0 + } + + /// Same as [`Self::get_mut()`], but uses custom divergence checks for the type encapsulated in the [`CheckDivergence`] trait. + pub fn get_custom_mut( + &mut self, + name: &str, + mut action: impl FnMut(ShadowMut<'_, Main, Shadow>) -> R, + ) -> R + where + R: CheckDivergence, + { + let main_output = action(ShadowMut::Main(self.main.as_mut())); + if let Some(shadow) = self.shadow.get_mut() { + let shadow_output = action(ShadowMut::Shadow(&mut shadow.vm)); + let errors = main_output.check_divergence(&shadow_output); + if let Err(err) = errors.into_result() { + self.report_shared(err.context(format!("get_mut({name})"))); + } + } + main_output + } } impl ShadowVm @@ -151,7 +360,6 @@ where } } -/// **Important.** This doesn't properly handle tracers; they are not passed to the shadow VM! impl VmInterface for ShadowVm where S: ReadStorage, @@ -197,9 +405,7 @@ where let main_result = self.main.inspect(main_tracer, execution_mode); if let Some(shadow) = self.shadow.get_mut() { let shadow_result = shadow.vm.inspect(shadow_tracer, execution_mode); - let mut errors = DivergenceErrors::new(); - errors.check_results_match(&main_result, &shadow_result); - + let errors = main_result.check_divergence(&shadow_result); if let Err(err) = errors.into_result() { let ctx = format!("executing VM with mode {execution_mode:?}"); self.report(err.context(ctx)); @@ -240,8 +446,7 @@ where tx, with_compression, ); - let mut errors = DivergenceErrors::new(); - errors.check_results_match(&main_tx_result, &shadow_result.1); + let errors = main_tx_result.check_divergence(&shadow_result.1); if let Err(err) = errors.into_result() { let ctx = format!( "inspecting transaction {tx_repr}, with_compression={with_compression:?}" @@ -256,31 +461,7 @@ where let main_batch = self.main.finish_batch(); if let Some(shadow) = self.shadow.get_mut() { let shadow_batch = shadow.vm.finish_batch(); - let mut errors = DivergenceErrors::new(); - errors.check_results_match( - &main_batch.block_tip_execution_result, - &shadow_batch.block_tip_execution_result, - ); - errors.check_final_states_match( - &main_batch.final_execution_state, - &shadow_batch.final_execution_state, - ); - errors.check_match( - "final_bootloader_memory", - &main_batch.final_bootloader_memory, - &shadow_batch.final_bootloader_memory, - ); - errors.check_match( - "pubdata_input", - &main_batch.pubdata_input, - &shadow_batch.pubdata_input, - ); - errors.check_match( - "state_diffs", - &main_batch.state_diffs, - &shadow_batch.state_diffs, - ); - + let errors = main_batch.check_divergence(&shadow_batch); if let Err(err) = errors.into_result() { self.report(err); } @@ -321,63 +502,15 @@ impl DivergenceErrors { } } + fn extend(&mut self, from: Self) { + self.divergences.extend(from.divergences); + } + fn context(mut self, context: String) -> Self { self.context = Some(context); self } - fn check_results_match( - &mut self, - main_result: &VmExecutionResultAndLogs, - shadow_result: &VmExecutionResultAndLogs, - ) { - self.check_match("result", &main_result.result, &shadow_result.result); - self.check_match( - "logs.events", - &main_result.logs.events, - &shadow_result.logs.events, - ); - self.check_match( - "logs.system_l2_to_l1_logs", - &main_result.logs.system_l2_to_l1_logs, - &shadow_result.logs.system_l2_to_l1_logs, - ); - self.check_match( - "logs.user_l2_to_l1_logs", - &main_result.logs.user_l2_to_l1_logs, - &shadow_result.logs.user_l2_to_l1_logs, - ); - let main_logs = UniqueStorageLogs::new(&main_result.logs.storage_logs); - let shadow_logs = UniqueStorageLogs::new(&shadow_result.logs.storage_logs); - self.check_match("logs.storage_logs", &main_logs, &shadow_logs); - self.check_match("refunds", &main_result.refunds, &shadow_result.refunds); - self.check_match( - "statistics.circuit_statistic", - &main_result.statistics.circuit_statistic, - &shadow_result.statistics.circuit_statistic, - ); - self.check_match( - "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) { if main != shadow { let comparison = pretty_assertions::Comparison::new(main, shadow); @@ -386,47 +519,6 @@ impl DivergenceErrors { } } - fn check_final_states_match( - &mut self, - main: &CurrentExecutionState, - shadow: &CurrentExecutionState, - ) { - self.check_match("final_state.events", &main.events, &shadow.events); - self.check_match( - "final_state.user_l2_to_l1_logs", - &main.user_l2_to_l1_logs, - &shadow.user_l2_to_l1_logs, - ); - self.check_match( - "final_state.system_logs", - &main.system_logs, - &shadow.system_logs, - ); - self.check_match( - "final_state.storage_refunds", - &main.storage_refunds, - &shadow.storage_refunds, - ); - self.check_match( - "final_state.pubdata_costs", - &main.pubdata_costs, - &shadow.pubdata_costs, - ); - self.check_match( - "final_state.used_contract_hashes", - &main.used_contract_hashes.iter().collect::>(), - &shadow.used_contract_hashes.iter().collect::>(), - ); - - let main_deduplicated_logs = Self::gather_logs(&main.deduplicated_storage_logs); - let shadow_deduplicated_logs = Self::gather_logs(&shadow.deduplicated_storage_logs); - self.check_match( - "deduplicated_storage_logs", - &main_deduplicated_logs, - &shadow_deduplicated_logs, - ); - } - fn gather_logs(logs: &[StorageLog]) -> BTreeMap { logs.iter() .filter(|log| log.is_write())