diff --git a/core/lib/multivm/src/interface/types/outputs/l2_block.rs b/core/lib/multivm/src/interface/types/outputs/l2_block.rs index ccbcba15f654..6125b2742d15 100644 --- a/core/lib/multivm/src/interface/types/outputs/l2_block.rs +++ b/core/lib/multivm/src/interface/types/outputs/l2_block.rs @@ -1,5 +1,6 @@ use zksync_types::H256; +#[derive(Debug)] pub struct L2Block { pub number: u32, pub timestamp: u64, diff --git a/core/lib/multivm/src/versions/shadow.rs b/core/lib/multivm/src/versions/shadow.rs index e3e106defdaf..f8554ed6a8eb 100644 --- a/core/lib/multivm/src/versions/shadow.rs +++ b/core/lib/multivm/src/versions/shadow.rs @@ -1,4 +1,7 @@ -use std::{collections::BTreeMap, fmt}; +use std::{ + collections::{BTreeMap, HashSet}, + fmt, +}; use anyhow::Context as _; use zksync_state::{ImmutableStorageView, ReadStorage, StoragePtr, StorageView}; @@ -276,6 +279,21 @@ impl DivergenceErrors { &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); diff --git a/core/lib/multivm/src/versions/vm_fast/tests/code_oracle.rs b/core/lib/multivm/src/versions/vm_fast/tests/code_oracle.rs index 419b54fc8548..24fda3beed4b 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/code_oracle.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/code_oracle.rs @@ -1,6 +1,8 @@ use ethabi::Token; -use zksync_types::{get_known_code_key, web3::keccak256, Address, Execute, U256}; -use zksync_utils::{bytecode::hash_bytecode, u256_to_h256}; +use zksync_types::{ + get_known_code_key, web3::keccak256, Address, Execute, StorageLogWithPreviousValue, U256, +}; +use zksync_utils::{bytecode::hash_bytecode, h256_to_u256, u256_to_h256}; use crate::{ interface::{TxExecutionMode, VmExecutionMode, VmInterface}, @@ -68,7 +70,10 @@ fn test_code_oracle() { vm.vm.push_transaction(tx1); let result = vm.vm.execute(VmExecutionMode::OneTx); - assert!(!result.result.is_failed(), "Transaction wasn't successful"); + assert!( + !result.result.is_failed(), + "Transaction wasn't successful: {result:#?}" + ); // Now, we ask for the same bytecode. We use to partially check whether the memory page with // the decommitted bytecode gets erased (it shouldn't). @@ -88,7 +93,21 @@ fn test_code_oracle() { ); vm.vm.push_transaction(tx2); let result = vm.vm.execute(VmExecutionMode::OneTx); - assert!(!result.result.is_failed(), "Transaction wasn't successful"); + assert!( + !result.result.is_failed(), + "Transaction wasn't successful: {result:#?}" + ); +} + +fn find_code_oracle_cost_log( + precompiles_contract_address: Address, + logs: &[StorageLogWithPreviousValue], +) -> &StorageLogWithPreviousValue { + logs.iter() + .find(|log| { + *log.log.key.address() == precompiles_contract_address && log.log.key.key().is_zero() + }) + .expect("no code oracle cost log") } #[test] @@ -145,5 +164,88 @@ fn test_code_oracle_big_bytecode() { vm.vm.push_transaction(tx1); let result = vm.vm.execute(VmExecutionMode::OneTx); - assert!(!result.result.is_failed(), "Transaction wasn't successful"); + assert!( + !result.result.is_failed(), + "Transaction wasn't successful: {result:#?}" + ); +} + +#[test] +fn refunds_in_code_oracle() { + let precompiles_contract_address = Address::random(); + let precompile_contract_bytecode = read_precompiles_contract(); + + let normal_zkevm_bytecode = read_test_contract(); + let normal_zkevm_bytecode_hash = hash_bytecode(&normal_zkevm_bytecode); + let normal_zkevm_bytecode_keccak_hash = keccak256(&normal_zkevm_bytecode); + let mut storage = get_empty_storage(); + storage.set_value( + get_known_code_key(&normal_zkevm_bytecode_hash), + u256_to_h256(U256::one()), + ); + + let precompile_contract = load_precompiles_contract(); + let call_code_oracle_function = precompile_contract.function("callCodeOracle").unwrap(); + + // Execute code oracle twice with identical VM state that only differs in that the queried bytecode + // is already decommitted the second time. The second call must consume less gas (`decommit` doesn't charge additional gas + // for already decommitted codes). + let mut oracle_costs = vec![]; + for decommit in [false, true] { + let mut vm = VmTesterBuilder::new() + .with_execution_mode(TxExecutionMode::VerifyExecute) + .with_random_rich_accounts(1) + .with_custom_contracts(vec![( + precompile_contract_bytecode.clone(), + precompiles_contract_address, + false, + )]) + .with_storage(storage.clone()) + .build(); + + vm.vm.insert_bytecodes([normal_zkevm_bytecode.as_slice()]); + + let account = &mut vm.rich_accounts[0]; + if decommit { + let (_, is_fresh) = vm + .vm + .inner + .world_diff + .decommit_opcode(&mut vm.vm.world, h256_to_u256(normal_zkevm_bytecode_hash)); + assert!(is_fresh); + } + + let tx = account.get_l2_tx_for_execute( + Execute { + contract_address: precompiles_contract_address, + calldata: call_code_oracle_function + .encode_input(&[ + Token::FixedBytes(normal_zkevm_bytecode_hash.0.to_vec()), + Token::FixedBytes(normal_zkevm_bytecode_keccak_hash.to_vec()), + ]) + .unwrap(), + value: U256::zero(), + factory_deps: vec![], + }, + None, + ); + + vm.vm.push_transaction(tx); + let result = vm.vm.execute(VmExecutionMode::OneTx); + assert!( + !result.result.is_failed(), + "Transaction wasn't successful: {result:#?}" + ); + let log = + find_code_oracle_cost_log(precompiles_contract_address, &result.logs.storage_logs); + oracle_costs.push(log.log.value); + } + + // The refund is equal to `gasCost` parameter passed to the `decommit` opcode, which is defined as `4 * contract_length_in_words` + // in `CodeOracle.yul`. + let code_oracle_refund = h256_to_u256(oracle_costs[0]) - h256_to_u256(oracle_costs[1]); + assert_eq!( + code_oracle_refund, + (4 * (normal_zkevm_bytecode.len() / 32)).into() + ); } diff --git a/core/lib/multivm/src/versions/vm_fast/tests/default_aa.rs b/core/lib/multivm/src/versions/vm_fast/tests/default_aa.rs index a884774c2377..460c8251652b 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/default_aa.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/default_aa.rs @@ -8,13 +8,11 @@ use zksync_utils::u256_to_h256; use crate::{ interface::{TxExecutionMode, VmExecutionMode, VmInterface}, - vm_fast::{ - tests::{ - tester::{DeployContractsTx, TxType, VmTesterBuilder}, - utils::{get_balance, read_test_contract, verify_required_storage}, - }, - utils::fee::get_batch_base_fee, + vm_fast::tests::{ + tester::{DeployContractsTx, TxType, VmTesterBuilder}, + utils::{get_balance, read_test_contract, verify_required_storage}, }, + vm_latest::utils::fee::get_batch_base_fee, }; #[test] @@ -54,13 +52,17 @@ fn test_default_aa_interaction() { // The contract should be deployed successfully. let account_code_key = get_code_key(&address); - let expected_slots = vec![ + let expected_slots = [ (u256_to_h256(expected_nonce), account_nonce_key), (u256_to_h256(U256::from(1u32)), known_codes_key), (bytecode_hash, account_code_key), ]; - verify_required_storage(&vm.vm.state, expected_slots); + verify_required_storage( + &expected_slots, + &mut vm.vm.world.storage, + vm.vm.inner.world_diff.get_storage_state(), + ); let expected_fee = maximal_fee - U256::from(result.refunds.gas_refunded) @@ -68,7 +70,8 @@ fn test_default_aa_interaction() { let operator_balance = get_balance( AccountTreeId::new(L2_BASE_TOKEN_ADDRESS), &vm.fee_account, - vm.vm.state.storage.storage.get_ptr(), + &mut vm.vm.world.storage, + vm.vm.inner.world_diff.get_storage_state(), ); assert_eq!( diff --git a/core/lib/multivm/src/versions/vm_fast/tests/get_used_contracts.rs b/core/lib/multivm/src/versions/vm_fast/tests/get_used_contracts.rs index dc6326dd6352..af90566671ee 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/get_used_contracts.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/get_used_contracts.rs @@ -38,12 +38,12 @@ fn test_get_used_contracts() { assert!(vm .vm - .get_decommitted_hashes() + .decommitted_hashes() .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.get_decommitted_hashes().collect::>(), + vm.vm.decommitted_hashes().collect::>(), known_bytecodes_without_aa_code(&vm.vm) ); @@ -78,7 +78,7 @@ fn test_get_used_contracts() { let hash = hash_bytecode(&factory_dep); let hash_to_u256 = h256_to_u256(hash); assert!(known_bytecodes_without_aa_code(&vm.vm).contains(&hash_to_u256)); - assert!(!vm.vm.get_decommitted_hashes().contains(&hash_to_u256)); + assert!(!vm.vm.decommitted_hashes().contains(&hash_to_u256)); } } diff --git a/core/lib/multivm/src/versions/vm_fast/tests/l1_tx_execution.rs b/core/lib/multivm/src/versions/vm_fast/tests/l1_tx_execution.rs index 5c751bf8837a..033a7b2658fa 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/l1_tx_execution.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/l1_tx_execution.rs @@ -20,7 +20,6 @@ use crate::{ }, }; -#[ignore] // FIXME: fails on `assert_eq!(res.initial_storage_writes, basic_initial_writes)` #[test] fn test_l1_tx_execution() { // In this test, we try to execute a contract deployment from L1 diff --git a/core/lib/multivm/src/versions/vm_fast/tests/mod.rs b/core/lib/multivm/src/versions/vm_fast/tests/mod.rs index 51d17c194fdc..9d5b229f23a9 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/mod.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/mod.rs @@ -1,20 +1,18 @@ mod bootloader; -//mod default_aa; -// TODO - fix this test -// `mod invalid_bytecode;` -//mod block_tip; +mod default_aa; +//mod block_tip; FIXME: requires vm metrics mod bytecode_publishing; -// mod call_tracer; -// mod circuits; +// mod call_tracer; FIXME: requires tracers +// mod circuits; FIXME: requires tracers / circuit stats mod code_oracle; mod gas_limit; mod get_used_contracts; mod is_write_initial; mod l1_tx_execution; mod l2_blocks; -//mod nonce_holder; -// mod precompiles; -// mod prestate_tracer; +mod nonce_holder; +// mod precompiles; FIXME: requires tracers / circuit stats +// mod prestate_tracer; FIXME: is pre-state tracer still relevant? mod refunds; mod require_eip712; mod rollbacks; @@ -24,5 +22,5 @@ mod storage; mod tester; mod tracing_execution_error; mod transfer; -// mod upgrade; +mod upgrade; mod utils; diff --git a/core/lib/multivm/src/versions/vm_fast/tests/nonce_holder.rs b/core/lib/multivm/src/versions/vm_fast/tests/nonce_holder.rs index 7dce4d36a7cd..b18676cf2ba6 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/nonce_holder.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/nonce_holder.rs @@ -1,16 +1,13 @@ -use zksync_types::{Execute, Nonce}; +use zksync_types::{Execute, ExecuteTransactionCommon, Nonce}; use crate::{ interface::{ ExecutionResult, Halt, TxExecutionMode, TxRevertReason, VmExecutionMode, VmInterface, VmRevertReason, }, - vm_fast::{ - tests::{ - tester::{Account, VmTesterBuilder}, - utils::read_nonce_holder_tester, - }, - transaction_data::TransactionData, + vm_fast::tests::{ + tester::{Account, VmTesterBuilder}, + utils::read_nonce_holder_tester, }, }; @@ -60,21 +57,21 @@ fn test_nonce_holder() { // it will fail again and again. At the same time we have to keep the same storage, because we want to keep the nonce holder contract state. // The easiest way in terms of lifetimes is to reuse `vm_builder` to achieve it. vm.reset_state(true); - let mut transaction_data: TransactionData = account - .get_l2_tx_for_execute_with_nonce( - Execute { - contract_address: account.address, - calldata: vec![12], - value: Default::default(), - factory_deps: None, - }, - None, - Nonce(nonce), - ) - .into(); - - transaction_data.signature = vec![test_mode.into()]; - vm.vm.push_raw_transaction(transaction_data, 0, 0, true); + let mut transaction = account.get_l2_tx_for_execute_with_nonce( + Execute { + contract_address: account.address, + calldata: vec![12], + value: Default::default(), + factory_deps: vec![], + }, + None, + Nonce(nonce), + ); + let ExecuteTransactionCommon::L2(tx_data) = &mut transaction.common_data else { + unreachable!(); + }; + tx_data.signature = vec![test_mode.into()]; + vm.vm.push_transaction_inner(transaction, 0, true); let result = vm.vm.execute(VmExecutionMode::OneTx); if let Some(msg) = error_message { @@ -86,14 +83,9 @@ fn test_nonce_holder() { let ExecutionResult::Halt { reason } = result.result else { panic!("Expected revert, got {:?}", result.result); }; - assert_eq!( - reason.to_string(), - expected_error.to_string(), - "{}", - comment - ); + assert_eq!(reason.to_string(), expected_error.to_string(), "{comment}"); } else { - assert!(!result.result.is_failed(), "{}", comment); + assert!(!result.result.is_failed(), "{comment}: {result:?}"); } }; // Test 1: trying to set value under non sequential nonce value. diff --git a/core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs b/core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs index 97f929927257..7e378a2b62c4 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs @@ -23,15 +23,20 @@ impl VmTester { AccountTreeId::new(L2_BASE_TOKEN_ADDRESS), &address, ); - h256_to_u256(self.vm.world.storage.read_value(&key)) + self.vm + .inner + .world_diff + .get_storage_state() + .get(&(L2_BASE_TOKEN_ADDRESS, h256_to_u256(*key.key()))) + .copied() + .unwrap_or_else(|| h256_to_u256(self.vm.world.storage.read_value(&key))) } } -#[ignore] // FIXME: fails on `assert_eq!(vm.get_eth_balance(beneficiary.address), U256::from(888000088))` -#[tokio::test] /// This test deploys 'buggy' account abstraction code, and then tries accessing it both with legacy /// and EIP712 transactions. /// Currently we support both, but in the future, we should allow only EIP712 transactions to access the AA accounts. +#[tokio::test] async fn test_require_eip712() { // Use 3 accounts: // - `private_address` - EOA account, where we have the key @@ -134,7 +139,8 @@ async fn test_require_eip712() { Default::default(), ); - let transaction_request: TransactionRequest = tx_712.into(); + let mut transaction_request: TransactionRequest = tx_712.into(); + transaction_request.chain_id = Some(chain_id.into()); let domain = Eip712Domain::new(L2ChainId::from(chain_id)); let signature = private_account diff --git a/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs b/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs index 07c9d3eaa252..7715dd0a6d49 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/tester/vm_tester.rs @@ -1,5 +1,6 @@ use std::{cell::RefCell, rc::Rc}; +use vm2::WorldDiff; use zksync_contracts::BaseSystemContracts; use zksync_state::{InMemoryStorage, StoragePtr}; use zksync_test_account::{Account, TxType}; @@ -9,7 +10,8 @@ use zksync_types::{ get_code_key, get_is_account_key, helpers::unix_timestamp_ms, utils::{deployed_address_create, storage_key_for_eth_balance}, - Address, L1BatchNumber, L2BlockNumber, L2ChainId, Nonce, ProtocolVersionId, U256, + AccountTreeId, Address, L1BatchNumber, L2BlockNumber, L2ChainId, Nonce, ProtocolVersionId, + StorageKey, U256, }; use zksync_utils::{bytecode::hash_bytecode, u256_to_h256}; @@ -26,6 +28,7 @@ pub(crate) struct VmTester { pub(crate) storage: StoragePtr, pub(crate) deployer: Option, pub(crate) test_contract: Option
, + pub(crate) fee_account: Address, pub(crate) rich_accounts: Vec, pub(crate) custom_contracts: Vec, } @@ -49,6 +52,7 @@ impl VmTester { pub(crate) fn reset_with_empty_storage(&mut self) { self.storage = Rc::new(RefCell::new(get_empty_storage())); + self.vm.inner.world_diff = WorldDiff::default(); self.reset_state(false); } @@ -69,9 +73,19 @@ impl VmTester { // `insert_contracts(&mut self.storage, &self.custom_contracts);` } + let storage = self.storage.clone(); + { + let mut storage = storage.borrow_mut(); + // Commit pending storage changes (old VM versions commit them on successful execution) + for (&(address, slot), &value) in self.vm.inner.world_diff.get_storage_state() { + let key = StorageKey::new(AccountTreeId::new(address), u256_to_h256(slot)); + storage.set_value(key, u256_to_h256(value)); + } + } + let mut l1_batch = self.vm.batch_env.clone(); if use_latest_l2_block { - let last_l2_block = load_last_l2_block(self.storage.clone()).unwrap_or(L2Block { + let last_l2_block = load_last_l2_block(&storage).unwrap_or(L2Block { number: 0, timestamp: 0, hash: L2BlockHasher::legacy_hash(L2BlockNumber(0)), @@ -84,12 +98,11 @@ impl VmTester { }; } - let vm = Vm::new(l1_batch, self.vm.system_env.clone(), self.storage.clone()); + let vm = Vm::new(l1_batch, self.vm.system_env.clone(), storage); if self.test_contract.is_some() { self.deploy_test_contract(); } - self.vm = vm; } } @@ -216,6 +229,7 @@ impl VmTesterBuilder { make_account_rich(storage_ptr.clone(), deployer); } + let fee_account = l1_batch_env.fee_account; let vm = Vm::new(l1_batch_env, self.system_env, storage_ptr.clone()); VmTester { @@ -223,6 +237,7 @@ impl VmTesterBuilder { storage: storage_ptr, deployer: self.deployer, test_contract: None, + fee_account, rich_accounts: self.rich_accounts.clone(), custom_contracts: self.custom_contracts.clone(), } diff --git a/core/lib/multivm/src/versions/vm_fast/tests/upgrade.rs b/core/lib/multivm/src/versions/vm_fast/tests/upgrade.rs index 78412c4c231b..616436776090 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/upgrade.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/upgrade.rs @@ -1,5 +1,4 @@ use zksync_contracts::{deployer_contract, load_sys_contract, read_bytecode}; -use zksync_state::WriteStorage; use zksync_test_account::TxType; use zksync_types::{ ethabi::{Contract, Token}, @@ -9,9 +8,8 @@ use zksync_types::{ CONTRACT_DEPLOYER_ADDRESS, CONTRACT_FORCE_DEPLOYER_ADDRESS, H160, H256, REQUIRED_L1_TO_L2_GAS_PER_PUBDATA_BYTE, U256, }; -use zksync_utils::{bytecode::hash_bytecode, bytes_to_be_words, h256_to_u256, u256_to_h256}; +use zksync_utils::{bytecode::hash_bytecode, u256_to_h256}; -use super::utils::{get_complex_upgrade_abi, read_test_contract}; use crate::{ interface::{ ExecutionResult, Halt, TxExecutionMode, VmExecutionMode, VmInterface, @@ -19,7 +17,10 @@ use crate::{ }, vm_fast::tests::{ tester::VmTesterBuilder, - utils::{read_complex_upgrade, verify_required_storage}, + utils::{ + get_complex_upgrade_abi, read_complex_upgrade, read_test_contract, + verify_required_storage, + }, }, }; @@ -35,8 +36,7 @@ fn test_protocol_upgrade_is_first() { .build(); let bytecode_hash = hash_bytecode(&read_test_contract()); - vm.vm - .storage + vm.storage .borrow_mut() .set_value(get_known_code_key(&bytecode_hash), u256_to_h256(1.into())); @@ -158,10 +158,14 @@ fn test_force_deploy_upgrade() { "The force upgrade was not successful" ); - let expected_slots = vec![(bytecode_hash, get_code_key(&address_to_deploy))]; + let expected_slots = [(bytecode_hash, get_code_key(&address_to_deploy))]; // Verify that the bytecode has been set correctly - verify_required_storage(&vm.vm.state, expected_slots); + verify_required_storage( + &expected_slots, + &mut *vm.storage.borrow_mut(), + vm.vm.inner.world_diff.get_storage_state(), + ); } /// Here we show how the work with the complex upgrader could be done @@ -173,8 +177,6 @@ fn test_complex_upgrader() { .with_random_rich_accounts(1) .build(); - let storage_view = vm.storage.clone(); - let bytecode_hash = hash_bytecode(&read_complex_upgrade()); let msg_sender_test_hash = hash_bytecode(&read_msg_sender_test()); @@ -183,28 +185,17 @@ fn test_complex_upgrader() { let upgrade_impl = H160::random(); let account_code_key = get_code_key(&upgrade_impl); - storage_view - .borrow_mut() - .set_value(get_known_code_key(&bytecode_hash), u256_to_h256(1.into())); - storage_view.borrow_mut().set_value( - get_known_code_key(&msg_sender_test_hash), - u256_to_h256(1.into()), - ); - storage_view - .borrow_mut() - .set_value(account_code_key, bytecode_hash); - drop(storage_view); - - vm.vm.state.decommittment_processor.populate(vec![ - ( - h256_to_u256(bytecode_hash), - bytes_to_be_words(read_complex_upgrade()), - ), - ( - h256_to_u256(msg_sender_test_hash), - bytes_to_be_words(read_msg_sender_test()), - ), - ]); + { + let mut storage = vm.storage.borrow_mut(); + storage.set_value(get_known_code_key(&bytecode_hash), u256_to_h256(1.into())); + storage.set_value( + get_known_code_key(&msg_sender_test_hash), + u256_to_h256(1.into()), + ); + storage.set_value(account_code_key, bytecode_hash); + storage.store_factory_dep(bytecode_hash, read_complex_upgrade()); + storage.store_factory_dep(msg_sender_test_hash, read_msg_sender_test()); + } let address_to_deploy1 = H160::random(); let address_to_deploy2 = H160::random(); @@ -223,13 +214,17 @@ fn test_complex_upgrader() { "The force upgrade was not successful" ); - let expected_slots = vec![ + let expected_slots = [ (bytecode_hash, get_code_key(&address_to_deploy1)), (bytecode_hash, get_code_key(&address_to_deploy2)), ]; // Verify that the bytecode has been set correctly - verify_required_storage(&vm.vm.state, expected_slots); + verify_required_storage( + &expected_slots, + &mut *vm.storage.borrow_mut(), + vm.vm.inner.world_diff.get_storage_state(), + ); } #[derive(Debug, Clone)] @@ -272,7 +267,7 @@ fn get_forced_deploy_tx(deployment: &[ForceDeployment]) -> Transaction { let execute = Execute { contract_address: CONTRACT_DEPLOYER_ADDRESS, calldata, - factory_deps: None, + factory_deps: vec![], value: U256::zero(), }; @@ -322,7 +317,7 @@ fn get_complex_upgrade_tx( let execute = Execute { contract_address: COMPLEX_UPGRADER_ADDRESS, calldata: complex_upgrader_calldata, - factory_deps: None, + factory_deps: vec![], value: U256::zero(), }; diff --git a/core/lib/multivm/src/versions/vm_fast/tests/utils.rs b/core/lib/multivm/src/versions/vm_fast/tests/utils.rs index f5217a0e80ac..abb8aa17d6fd 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/utils.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/utils.rs @@ -8,9 +8,10 @@ use zksync_contracts::{ }; use zksync_state::ReadStorage; use zksync_types::{ - utils::storage_key_for_standard_token_balance, AccountTreeId, Address, H160, U256, + utils::storage_key_for_standard_token_balance, AccountTreeId, Address, StorageKey, H160, H256, + U256, }; -use zksync_utils::{bytecode::hash_bytecode, bytes_to_be_words, h256_to_u256}; +use zksync_utils::{bytecode::hash_bytecode, bytes_to_be_words, h256_to_u256, u256_to_h256}; pub(crate) static BASE_SYSTEM_CONTRACTS: Lazy = Lazy::new(BaseSystemContracts::load_from_disk); @@ -22,6 +23,24 @@ pub(crate) fn verify_required_memory(state: &State, required_values: Vec<(U256, } } +pub(crate) fn verify_required_storage( + required_values: &[(H256, StorageKey)], + main_storage: &mut impl ReadStorage, + storage_changes: &BTreeMap<(H160, U256), U256>, +) { + for &(required_value, key) in required_values { + let current_value = storage_changes + .get(&(*key.account().address(), h256_to_u256(*key.key()))) + .copied() + .unwrap_or_else(|| h256_to_u256(main_storage.read_value(&key))); + + assert_eq!( + u256_to_h256(current_value), + required_value, + "Invalid value at key {key:?}" + ); + } +} pub(crate) fn get_balance( token_id: AccountTreeId, account: &Address, @@ -32,7 +51,7 @@ pub(crate) fn get_balance( storage_changes .get(&(*key.account().address(), h256_to_u256(*key.key()))) - .cloned() + .copied() .unwrap_or_else(|| h256_to_u256(main_storage.read_value(&key))) } @@ -87,3 +106,17 @@ pub(crate) fn load_precompiles_contract() -> Contract { "etc/contracts-test-data/artifacts-zk/contracts/precompiles/precompiles.sol/Precompiles.json", ) } + +pub(crate) fn read_nonce_holder_tester() -> Vec { + read_bytecode("etc/contracts-test-data/artifacts-zk/contracts/custom-account/nonce-holder-test.sol/NonceHolderTest.json") +} + +pub(crate) fn read_complex_upgrade() -> Vec { + read_bytecode("etc/contracts-test-data/artifacts-zk/contracts/complex-upgrade/complex-upgrade.sol/ComplexUpgrade.json") +} + +pub(crate) fn get_complex_upgrade_abi() -> Contract { + load_contract( + "etc/contracts-test-data/artifacts-zk/contracts/complex-upgrade/complex-upgrade.sol/ComplexUpgrade.json" + ) +} diff --git a/core/lib/multivm/src/versions/vm_fast/vm.rs b/core/lib/multivm/src/versions/vm_fast/vm.rs index dd599d471b3b..c6604dff7449 100644 --- a/core/lib/multivm/src/versions/vm_fast/vm.rs +++ b/core/lib/multivm/src/versions/vm_fast/vm.rs @@ -185,8 +185,7 @@ impl Vm { assert!(fp.offset == 0); let return_data = self.inner.state.heaps[fp.memory_page] - .read_range_big_endian(fp.start..fp.start + fp.length) - .to_vec(); + .read_range_big_endian(fp.start..fp.start + fp.length); last_tx_result = Some(if result.is_zero() { ExecutionResult::Revert { @@ -418,8 +417,7 @@ impl Vm { ) } - #[cfg(test)] - pub(crate) fn get_decommitted_hashes(&self) -> impl Iterator + '_ { + pub(crate) fn decommitted_hashes(&self) -> impl Iterator + '_ { self.inner.world_diff.decommitted_hashes() } } @@ -503,75 +501,76 @@ impl VmInterface for Vm { // Move the pointer to the next transaction self.bootloader_state.move_tx_to_execute_pointer(); track_refunds = true; - // Create a snapshot to roll back the bootloader call frame on halt. - self.make_snapshot(); } let start = self.inner.world_diff.snapshot(); let pubdata_before = self.inner.world_diff.pubdata(); let (result, refunds) = self.run(execution_mode, track_refunds); - if matches!(execution_mode, VmExecutionMode::OneTx) { - if matches!(result, ExecutionResult::Halt { .. }) { - // Only `Halt`ed executions need a rollback; `Revert`s are correctly handled by the bootloader - // so the bootloader / system contract state should be persisted. - self.rollback_to_the_latest_snapshot(); - } else { - self.pop_snapshot_no_rollback(); - } - } + let ignore_world_diff = matches!(execution_mode, VmExecutionMode::OneTx) + && matches!(result, ExecutionResult::Halt { .. }); - let events = merge_events( - self.inner.world_diff.events_after(&start), - self.batch_env.number, - ); - let user_l2_to_l1_logs = extract_l2tol1logs_from_l1_messenger(&events) - .into_iter() - .map(Into::into) - .map(UserL2ToL1Log) - .collect(); - let pubdata_after = self.inner.world_diff.pubdata(); - - VmExecutionResultAndLogs { - result, - logs: VmExecutionLogs { - storage_logs: self - .inner - .world_diff - .get_storage_changes_after(&start) - .map(|((address, key), change)| StorageLogWithPreviousValue { - log: StorageLog { - key: StorageKey::new(AccountTreeId::new(address), u256_to_h256(key)), - value: u256_to_h256(change.after), - kind: if change.is_initial { - StorageLogKind::InitialWrite - } else { - StorageLogKind::RepeatedWrite - }, + // 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. + let logs = if ignore_world_diff { + VmExecutionLogs::default() + } else { + let storage_logs = self + .inner + .world_diff + .get_storage_changes_after(&start) + .map(|((address, key), change)| StorageLogWithPreviousValue { + log: StorageLog { + key: StorageKey::new(AccountTreeId::new(address), u256_to_h256(key)), + value: u256_to_h256(change.after), + kind: if change.is_initial { + StorageLogKind::InitialWrite + } else { + StorageLogKind::RepeatedWrite }, - previous_value: u256_to_h256(change.before.unwrap_or_default()), - }) - .collect(), + }, + previous_value: u256_to_h256(change.before.unwrap_or_default()), + }) + .collect(); + let events = merge_events( + self.inner.world_diff.events_after(&start), + self.batch_env.number, + ); + let user_l2_to_l1_logs = extract_l2tol1logs_from_l1_messenger(&events) + .into_iter() + .map(Into::into) + .map(UserL2ToL1Log) + .collect(); + let system_l2_to_l1_logs = self + .inner + .world_diff + .l2_to_l1_logs_after(&start) + .iter() + .map(|x| x.glue_into()) + .collect(); + VmExecutionLogs { + storage_logs, events, user_l2_to_l1_logs, - system_l2_to_l1_logs: self - .inner - .world_diff - .l2_to_l1_logs_after(&start) - .iter() - .map(|x| x.glue_into()) - .collect(), + system_l2_to_l1_logs, total_log_queries_count: 0, // This field is unused - }, + } + }; + + let pubdata_after = self.inner.world_diff.pubdata(); + VmExecutionResultAndLogs { + result, + logs, + // FIXME (PLA-936): Fill statistics; investigate whether they should be zeroed on `Halt` statistics: VmExecutionStatistics { - contracts_used: 0, // TODO - cycles_used: 0, // TODO - gas_used: 0, // TODO - gas_remaining: 0, // TODO - computational_gas_used: 0, // TODO - total_log_queries: 0, // TODO + contracts_used: 0, + cycles_used: 0, + gas_used: 0, + gas_remaining: 0, + computational_gas_used: 0, + total_log_queries: 0, pubdata_published: (pubdata_after - pubdata_before).max(0) as u32, - circuit_statistic: Default::default(), // TODO + circuit_statistic: Default::default(), }, refunds, } @@ -612,8 +611,8 @@ impl VmInterface for Vm { } fn get_current_execution_state(&self) -> CurrentExecutionState { - // TODO fill all fields - let events = merge_events(self.inner.world_diff.events(), self.batch_env.number); + let world_diff = &self.inner.world_diff; + let events = merge_events(world_diff.events(), self.batch_env.number); let user_l2_to_l1_logs = extract_l2tol1logs_from_l1_messenger(&events) .into_iter() @@ -623,9 +622,7 @@ impl VmInterface for Vm { CurrentExecutionState { events, - deduplicated_storage_logs: self - .inner - .world_diff + deduplicated_storage_logs: world_diff .get_storage_changes() .map(|((address, key), (_, value))| StorageLog { key: StorageKey::new(AccountTreeId::new(address), u256_to_h256(key)), @@ -633,17 +630,15 @@ impl VmInterface for Vm { kind: StorageLogKind::RepeatedWrite, // Initialness doesn't matter here }) .collect(), - used_contract_hashes: vec![], - system_logs: self - .inner - .world_diff + used_contract_hashes: self.decommitted_hashes().collect(), + system_logs: world_diff .l2_to_l1_logs() .iter() .map(|x| x.glue_into()) .collect(), user_l2_to_l1_logs, - storage_refunds: vec![], - pubdata_costs: vec![], + storage_refunds: world_diff.storage_refunds().to_vec(), + pubdata_costs: world_diff.pubdata_costs().to_vec(), } } diff --git a/core/lib/multivm/src/versions/vm_latest/tests/code_oracle.rs b/core/lib/multivm/src/versions/vm_latest/tests/code_oracle.rs index 8c8c6e2d0970..7174e9be67de 100644 --- a/core/lib/multivm/src/versions/vm_latest/tests/code_oracle.rs +++ b/core/lib/multivm/src/versions/vm_latest/tests/code_oracle.rs @@ -1,6 +1,11 @@ use ethabi::Token; -use zk_evm_1_5_0::aux_structures::Timestamp; -use zksync_types::{get_known_code_key, web3::keccak256, Address, Execute, U256}; +use zk_evm_1_5_0::{ + aux_structures::{MemoryPage, Timestamp}, + zkevm_opcode_defs::{ContractCodeSha256Format, VersionedHashLen32}, +}; +use zksync_types::{ + get_known_code_key, web3::keccak256, Address, Execute, StorageLogWithPreviousValue, U256, +}; use zksync_utils::{bytecode::hash_bytecode, bytes_to_be_words, h256_to_u256, u256_to_h256}; use crate::{ @@ -79,7 +84,10 @@ fn test_code_oracle() { vm.vm.push_transaction(tx1); let result = vm.vm.execute(VmExecutionMode::OneTx); - assert!(!result.result.is_failed(), "Transaction wasn't successful"); + assert!( + !result.result.is_failed(), + "Transaction wasn't successful: {result:#?}" + ); // Now, we ask for the same bytecode. We use to partially check whether the memory page with // the decommitted bytecode gets erased (it shouldn't). @@ -99,7 +107,21 @@ fn test_code_oracle() { ); vm.vm.push_transaction(tx2); let result = vm.vm.execute(VmExecutionMode::OneTx); - assert!(!result.result.is_failed(), "Transaction wasn't successful"); + assert!( + !result.result.is_failed(), + "Transaction wasn't successful: {result:#?}" + ); +} + +fn find_code_oracle_cost_log( + precompiles_contract_address: Address, + logs: &[StorageLogWithPreviousValue], +) -> &StorageLogWithPreviousValue { + logs.iter() + .find(|log| { + *log.log.key.address() == precompiles_contract_address && log.log.key.key().is_zero() + }) + .expect("no code oracle cost log") } #[test] @@ -164,3 +186,97 @@ fn test_code_oracle_big_bytecode() { let result = vm.vm.execute(VmExecutionMode::OneTx); assert!(!result.result.is_failed(), "Transaction wasn't successful"); } + +#[test] +fn refunds_in_code_oracle() { + let precompiles_contract_address = Address::random(); + let precompile_contract_bytecode = read_precompiles_contract(); + + let normal_zkevm_bytecode = read_test_contract(); + let normal_zkevm_bytecode_hash = hash_bytecode(&normal_zkevm_bytecode); + let normal_zkevm_bytecode_keccak_hash = keccak256(&normal_zkevm_bytecode); + let normal_zkevm_bytecode_words = bytes_to_be_words(normal_zkevm_bytecode); + let mut storage = get_empty_storage(); + storage.set_value( + get_known_code_key(&normal_zkevm_bytecode_hash), + u256_to_h256(U256::one()), + ); + + let precompile_contract = load_precompiles_contract(); + let call_code_oracle_function = precompile_contract.function("callCodeOracle").unwrap(); + + // Execute code oracle twice with identical VM state that only differs in that the queried bytecode + // is already decommitted the second time. The second call must consume less gas (`decommit` doesn't charge additional gas + // for already decommitted codes). + let mut oracle_costs = vec![]; + for decommit in [false, true] { + let mut vm = VmTesterBuilder::new(HistoryEnabled) + .with_execution_mode(TxExecutionMode::VerifyExecute) + .with_random_rich_accounts(1) + .with_custom_contracts(vec![( + precompile_contract_bytecode.clone(), + precompiles_contract_address, + false, + )]) + .with_storage(storage.clone()) + .build(); + + vm.vm.state.decommittment_processor.populate( + vec![( + h256_to_u256(normal_zkevm_bytecode_hash), + normal_zkevm_bytecode_words.clone(), + )], + Timestamp(0), + ); + + let account = &mut vm.rich_accounts[0]; + if decommit { + let (header, normalized_preimage) = + ContractCodeSha256Format::normalize_for_decommitment(&normal_zkevm_bytecode_hash.0); + let query = vm + .vm + .state + .prepare_to_decommit( + 0, + header, + normalized_preimage, + MemoryPage(123), + Timestamp(0), + ) + .unwrap(); + + assert!(query.is_fresh); + vm.vm.state.execute_decommit(0, query).unwrap(); + } + + let tx = account.get_l2_tx_for_execute( + Execute { + contract_address: precompiles_contract_address, + calldata: call_code_oracle_function + .encode_input(&[ + Token::FixedBytes(normal_zkevm_bytecode_hash.0.to_vec()), + Token::FixedBytes(normal_zkevm_bytecode_keccak_hash.to_vec()), + ]) + .unwrap(), + value: U256::zero(), + factory_deps: vec![], + }, + None, + ); + + vm.vm.push_transaction(tx); + let result = vm.vm.execute(VmExecutionMode::OneTx); + assert!(!result.result.is_failed(), "Transaction wasn't successful"); + let log = + find_code_oracle_cost_log(precompiles_contract_address, &result.logs.storage_logs); + oracle_costs.push(log.log.value); + } + + // The refund is equal to `gasCost` parameter passed to the `decommit` opcode, which is defined as `4 * contract_length_in_words` + // in `CodeOracle.yul`. + let code_oracle_refund = h256_to_u256(oracle_costs[0]) - h256_to_u256(oracle_costs[1]); + assert_eq!( + code_oracle_refund, + (4 * normal_zkevm_bytecode_words.len()).into() + ); +} diff --git a/core/lib/multivm/src/versions/vm_latest/tests/tester/vm_tester.rs b/core/lib/multivm/src/versions/vm_latest/tests/tester/vm_tester.rs index 055d395c5b61..28d853486485 100644 --- a/core/lib/multivm/src/versions/vm_latest/tests/tester/vm_tester.rs +++ b/core/lib/multivm/src/versions/vm_latest/tests/tester/vm_tester.rs @@ -83,7 +83,7 @@ impl VmTester { let mut l1_batch = self.vm.batch_env.clone(); if use_latest_l2_block { - let last_l2_block = load_last_l2_block(self.storage.clone()).unwrap_or(L2Block { + let last_l2_block = load_last_l2_block(&self.storage).unwrap_or(L2Block { number: 0, timestamp: 0, hash: L2BlockHasher::legacy_hash(L2BlockNumber(0)), diff --git a/core/lib/multivm/src/versions/vm_latest/types/internals/vm_state.rs b/core/lib/multivm/src/versions/vm_latest/types/internals/vm_state.rs index 9a3e70f8dff6..b9ac0bfad229 100644 --- a/core/lib/multivm/src/versions/vm_latest/types/internals/vm_state.rs +++ b/core/lib/multivm/src/versions/vm_latest/types/internals/vm_state.rs @@ -63,7 +63,7 @@ pub(crate) fn new_vm_state( system_env: &SystemEnv, l1_batch_env: &L1BatchEnv, ) -> (ZkSyncVmState, BootloaderState) { - let last_l2_block = if let Some(last_l2_block) = load_last_l2_block(storage.clone()) { + let last_l2_block = if let Some(last_l2_block) = load_last_l2_block(&storage) { last_l2_block } else { // This is the scenario of either the first L2 block ever or diff --git a/core/lib/multivm/src/versions/vm_latest/utils/l2_blocks.rs b/core/lib/multivm/src/versions/vm_latest/utils/l2_blocks.rs index ec30a86013b9..d3253ffd7fb3 100644 --- a/core/lib/multivm/src/versions/vm_latest/utils/l2_blocks.rs +++ b/core/lib/multivm/src/versions/vm_latest/utils/l2_blocks.rs @@ -52,7 +52,7 @@ pub(crate) fn l2_block_hash( } /// Get last saved block from storage -pub fn load_last_l2_block(storage: StoragePtr) -> Option { +pub fn load_last_l2_block(storage: &StoragePtr) -> Option { // Get block number and timestamp let current_l2_block_info_key = StorageKey::new( AccountTreeId::new(SYSTEM_CONTEXT_ADDRESS), diff --git a/core/lib/state/src/storage_view.rs b/core/lib/state/src/storage_view.rs index 22bcdacbc547..b01f423f0787 100644 --- a/core/lib/state/src/storage_view.rs +++ b/core/lib/state/src/storage_view.rs @@ -224,8 +224,8 @@ impl WriteStorage for StorageView { } } -/// Immutable wrapper around [`StorageView`] that reads directly from the underlying storage ignoring any caching -/// or modifications in the [`StorageView`]. Used by the fast VM, which has its own internal management of writes and cache. +/// Immutable wrapper around [`StorageView`] that reads directly from the underlying storage ignoring any +/// modifications in the [`StorageView`]. Used by the fast VM, which has its own internal management of writes. #[derive(Debug)] pub struct ImmutableStorageView(StoragePtr>); @@ -236,24 +236,31 @@ impl ImmutableStorageView { } } +// All methods other than `read_value()` do not read back modified storage slots, so we proxy them as-is. impl ReadStorage for ImmutableStorageView { fn read_value(&mut self, key: &StorageKey) -> StorageValue { - self.0.borrow_mut().storage_handle.read_value(key) + let started_at = Instant::now(); + let mut this = self.0.borrow_mut(); + let cached_value = this.read_storage_keys().get(key); + cached_value.copied().unwrap_or_else(|| { + let value = this.storage_handle.read_value(key); + this.cache.read_storage_keys.insert(*key, value); + this.metrics.time_spent_on_storage_missed += started_at.elapsed(); + this.metrics.storage_invocations_missed += 1; + value + }) } fn is_write_initial(&mut self, key: &StorageKey) -> bool { - self.0.borrow_mut().storage_handle.is_write_initial(key) + self.0.borrow_mut().is_write_initial(key) } fn load_factory_dep(&mut self, hash: H256) -> Option> { - self.0.borrow_mut().storage_handle.load_factory_dep(hash) + self.0.borrow_mut().load_factory_dep(hash) } fn get_enumeration_index(&mut self, key: &StorageKey) -> Option { - self.0 - .borrow_mut() - .storage_handle - .get_enumeration_index(key) + self.0.borrow_mut().get_enumeration_index(key) } } @@ -305,4 +312,23 @@ mod test { assert_eq!(metrics.get_value_storage_invocations, 3); assert_eq!(metrics.set_value_storage_invocations, 2); } + + #[test] + fn immutable_storage_view() { + let account: AccountTreeId = AccountTreeId::new(Address::from([0xfe; 20])); + let key = H256::from_low_u64_be(61); + let value = H256::from_low_u64_be(73); + let key = StorageKey::new(account, key); + + let mut raw_storage = InMemoryStorage::default(); + raw_storage.set_value(key, value); + let storage_view = StorageView::new(raw_storage).to_rc_ptr(); + let mut immutable_view = ImmutableStorageView::new(storage_view.clone()); + + let new_value = H256::repeat_byte(0x11); + let prev_value = storage_view.borrow_mut().set_value(key, new_value); + assert_eq!(prev_value, value); + + assert_eq!(immutable_view.read_value(&key), value); + } } diff --git a/core/tests/ts-integration/tests/contracts.test.ts b/core/tests/ts-integration/tests/contracts.test.ts index 5ac87b71b684..e22385a1b278 100644 --- a/core/tests/ts-integration/tests/contracts.test.ts +++ b/core/tests/ts-integration/tests/contracts.test.ts @@ -431,8 +431,7 @@ describe('Smart contract behavior checks', () => { await expect(storageContract.assertTValue(0)).toBeAccepted([]); }); - // FIXME: doesn't work with new VM - test.skip('Should check code oracle works', async () => { + test('Should check code oracle works', async () => { // Deploy contract that calls CodeOracle. const artifact = require(`${ testMaster.environment().pathToHome