From f5efb0b90fb3154ad01dae003095d125a8e89fc0 Mon Sep 17 00:00:00 2001 From: George Mitenkov Date: Fri, 23 Aug 2024 14:59:51 +0100 Subject: [PATCH] [move][aptos-vm] End-to-end module publishing - MoveVM now provides an ability to stage modules into a temporary storage, verifying is this is "publishable". - Tests and test harnesses adapted to reduce the number of changes (compared to `main`), e.g., old publishing workflows are returned but marked as deprecated. - Tests adapted to work with both V1 and V2 loader publishing checks. - AptosVM now should correctly publish packages in non-block context and without custom verification extensions. --- .../src/module_and_script_storage/mod.rs | 3 - .../state_view_adapter.rs | 8 + .../temporary_module_storage.rs | 149 ------ .../aptos-vm-types/src/module_write_set.rs | 6 +- aptos-move/aptos-vm/src/aptos_vm.rs | 120 +++-- .../aptos-vm/src/move_vm_ext/session/mod.rs | 4 +- .../src/move_vm_ext/write_op_converter.rs | 4 +- .../aptos-vm/src/verifier/module_init.rs | 6 +- aptos-move/vm-genesis/src/lib.rs | 147 +++--- .../async/move-async-vm/src/async_vm.rs | 42 +- .../async/move-async-vm/tests/testsuite.rs | 90 +--- .../src/tests/bad_storage_tests.rs | 8 + .../src/tests/binary_format_version.rs | 190 ++++---- .../src/tests/instantiation_tests.rs | 63 ++- .../src/tests/loader_tests.rs | 153 +++--- .../src/tests/native_tests.rs | 128 +++-- .../src/tests/nested_loop_tests.rs | 32 +- third_party/move/move-vm/runtime/src/lib.rs | 7 +- .../move/move-vm/runtime/src/loader/mod.rs | 39 +- .../move/move-vm/runtime/src/runtime.rs | 140 ++---- .../move/move-vm/runtime/src/session.rs | 77 +-- .../move/move-vm/runtime/src/storage/dummy.rs | 9 + .../src/storage/implementations/mod.rs | 1 + .../unreachable_code_storage.rs | 99 ++++ .../implementations/unsync_code_storage.rs | 13 +- .../implementations/unsync_module_storage.rs | 28 +- .../move/move-vm/runtime/src/storage/mod.rs | 1 + .../runtime/src/storage/module_storage.rs | 19 + .../move-vm/runtime/src/storage/publishing.rs | 226 +++++++++ .../src/vm_test_harness.rs | 439 +++++++++--------- types/src/transaction/module.rs | 8 + 31 files changed, 1265 insertions(+), 994 deletions(-) delete mode 100644 aptos-move/aptos-vm-types/src/module_and_script_storage/temporary_module_storage.rs create mode 100644 third_party/move/move-vm/runtime/src/storage/implementations/unreachable_code_storage.rs create mode 100644 third_party/move/move-vm/runtime/src/storage/publishing.rs diff --git a/aptos-move/aptos-vm-types/src/module_and_script_storage/mod.rs b/aptos-move/aptos-vm-types/src/module_and_script_storage/mod.rs index 6c9f31b0daa2c6..5be7bee1a52d9d 100644 --- a/aptos-move/aptos-vm-types/src/module_and_script_storage/mod.rs +++ b/aptos-move/aptos-vm-types/src/module_and_script_storage/mod.rs @@ -6,6 +6,3 @@ pub mod script_storage; mod state_view_adapter; pub use state_view_adapter::{AptosCodeStorageAdapter, AsAptosCodeStorage}; - -mod temporary_module_storage; -pub use temporary_module_storage::TemporaryModuleStorage; diff --git a/aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs b/aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs index 5fdf159d60ac3b..84d0297b2e5d47 100644 --- a/aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs +++ b/aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs @@ -56,6 +56,14 @@ impl<'s, S: StateView> ModuleStorage for AptosCodeStorageAdapter<'s, S> { self.storage.check_module_exists(address, module_name) } + fn fetch_module_bytes( + &self, + address: &AccountAddress, + module_name: &IdentStr, + ) -> PartialVMResult> { + self.storage.fetch_module_bytes(address, module_name) + } + fn fetch_module_size_in_bytes( &self, address: &AccountAddress, diff --git a/aptos-move/aptos-vm-types/src/module_and_script_storage/temporary_module_storage.rs b/aptos-move/aptos-vm-types/src/module_and_script_storage/temporary_module_storage.rs deleted file mode 100644 index e2deaeb96927a1..00000000000000 --- a/aptos-move/aptos-vm-types/src/module_and_script_storage/temporary_module_storage.rs +++ /dev/null @@ -1,149 +0,0 @@ -// Copyright © Aptos Foundation -// SPDX-License-Identifier: Apache-2.0 - -use crate::module_write_set::ModuleWriteSet; -use aptos_types::{state_store::state_key::StateKey, write_set::WriteOp}; -use move_binary_format::{ - errors::{PartialVMError, PartialVMResult}, - CompiledModule, -}; -use move_core_types::{ - account_address::AccountAddress, identifier::IdentStr, metadata::Metadata, - vm_status::StatusCode, -}; -use move_vm_runtime::{Module, ModuleStorage, RuntimeEnvironment}; -use std::{collections::BTreeMap, sync::Arc}; - -/// A module storage with stashed temporary changes (write ops). Use by AptosVM to -/// process init_module, where published modules are made temporarily visible. -pub struct TemporaryModuleStorage<'a, M> { - runtime_env: &'a RuntimeEnvironment, - - // TODO(loader_v2): Implement cache which "caches" deserialization? Or we can construct - // Arc and store it inside MVHashMap instead! - write_ops: BTreeMap, - module_storage: &'a M, -} - -impl<'a, M: ModuleStorage> TemporaryModuleStorage<'a, M> { - /// Creates a new temporary module storage with stashed changes. - pub fn create( - runtime_env: &'a RuntimeEnvironment, - write_ops: BTreeMap, - module_storage: &'a M, - ) -> Self { - Self { - runtime_env, - write_ops, - module_storage, - } - } - - /// Destroys temporary module storage releasing the stshed changes. - pub fn destroy(self) -> ModuleWriteSet { - // We do not care here about writes to special addresses because there is no flushing. - ModuleWriteSet::new(false, self.write_ops) - } - - fn write_op_to_compiled_module( - &self, - write_op: &WriteOp, - ) -> PartialVMResult> { - // Modules can never be deleted, return an invariant violation for extra safety. - let bytes = write_op.bytes().ok_or_else(|| { - PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) - .with_message("Module deletion is not be possible".to_string()) - })?; - - let deserializer_config = &self.runtime_env.vm_config().deserializer_config; - Ok(Arc::new(CompiledModule::deserialize_with_config( - bytes, - deserializer_config, - )?)) - } -} - -impl<'a, M: ModuleStorage> ModuleStorage for TemporaryModuleStorage<'a, M> { - fn check_module_exists( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> PartialVMResult { - let state_key = StateKey::module(address, module_name); - if self.write_ops.contains_key(&state_key) { - return Ok(true); - } - self.module_storage - .check_module_exists(address, module_name) - } - - fn fetch_module_size_in_bytes( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> PartialVMResult { - let state_key = StateKey::module(address, module_name); - match self.write_ops.get(&state_key) { - Some(write_op) => Ok(write_op.size()), - None => self - .module_storage - .fetch_module_size_in_bytes(address, module_name), - } - } - - fn fetch_module_metadata( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> PartialVMResult> { - let deserialized_module = self.fetch_deserialized_module(address, module_name)?; - Ok(deserialized_module.metadata.clone()) - } - - fn fetch_deserialized_module( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> PartialVMResult> { - let state_key = StateKey::module(address, module_name); - match self.write_ops.get(&state_key) { - Some(write_op) => self.write_op_to_compiled_module(write_op), - None => self - .module_storage - .fetch_deserialized_module(address, module_name), - } - } - - fn fetch_verified_module( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> PartialVMResult> { - let state_key = StateKey::module(address, module_name); - match self.write_ops.get(&state_key) { - Some(write_op) => { - let compiled_module = self.write_op_to_compiled_module(write_op)?; - let partially_verified_module = self - .runtime_env - .build_partially_verified_module(compiled_module)?; - // TODO(loader_v2): Revisit this, technically by this point we have checked there are no cycles and all modules must be loaded. - let immediate_dependencies = partially_verified_module - .immediate_dependencies_iter() - .map(|(addr, name)| self.fetch_verified_module(addr, name)) - .collect::>>()?; - let verified_module = self - .runtime_env - .build_verified_module(partially_verified_module, &immediate_dependencies)?; - Ok(Arc::new(verified_module)) - }, - None => self - .module_storage - .fetch_verified_module(address, module_name), - } - } -} - -#[cfg(test)] -mod test { - // TODO(loader_v2): Implement tests. -} diff --git a/aptos-move/aptos-vm-types/src/module_write_set.rs b/aptos-move/aptos-vm-types/src/module_write_set.rs index a230c43780f6c9..bf310d3b2e65f9 100644 --- a/aptos-move/aptos-vm-types/src/module_write_set.rs +++ b/aptos-move/aptos-vm-types/src/module_write_set.rs @@ -72,8 +72,12 @@ impl ModuleWriteSet { self.has_writes_to_special_address } + pub fn is_empty(&self) -> bool { + self.write_ops().is_empty() + } + pub fn is_empty_or_invariant_violation(&self) -> PartialVMResult<()> { - if !self.write_ops().is_empty() { + if !self.is_empty() { return Err(PartialVMError::new( StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, )); diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index 1d42594ee209b7..cdda1753dfc882 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -78,7 +78,7 @@ use aptos_vm_types::{ environment::Environment, module_and_script_storage::{ module_storage::AptosModuleStorage, script_storage::AptosScriptStorage, - AptosCodeStorageAdapter, AsAptosCodeStorage, TemporaryModuleStorage, + AptosCodeStorageAdapter, AsAptosCodeStorage, }, module_write_set::ModuleWriteSet, output::VMOutput, @@ -111,6 +111,7 @@ use move_core_types::{ use move_vm_runtime::{ logging::expect_no_verification_errors, module_traversal::{TraversalContext, TraversalStorage}, + TemporaryModuleStorage, UnreachableCodeStorage, }; use move_vm_types::gas::{GasMeter, UnmeteredGasMeter}; use num_cpus; @@ -1452,11 +1453,10 @@ impl AptosVM { Ok(epilogue_session) } - /// Execute all module initializers. + /// Execute all module initializers. This code is only used for V1 loader implementation. fn execute_module_initialization( &self, session: &mut SessionExt, - module_storage: &impl AptosModuleStorage, gas_meter: &mut impl AptosGasMeter, modules: &[CompiledModule], exists: BTreeSet, @@ -1471,8 +1471,12 @@ impl AptosVM { continue; } *new_published_modules_loaded = true; - let init_function = - session.load_function(module_storage, &module.self_id(), init_func_name, &[]); + let init_function = session.load_function( + &UnreachableCodeStorage, + &module.self_id(), + init_func_name, + &[], + ); // it is ok to not have init_module function // init_module function should be (1) private and (2) has no return value // Note that for historic reasons, verification here is treated @@ -1491,7 +1495,7 @@ impl AptosVM { args, gas_meter, traversal_context, - module_storage, + &UnreachableCodeStorage, )?; } else { return Err(PartialVMError::new(StatusCode::CONSTRAINT_NOT_SATISFIED) @@ -1656,38 +1660,24 @@ impl AptosVM { let compat = Compatibility::new(check_struct_layout, check_friend_linking); if self.features().use_loader_v2() { - // TODO(loader_v2): This check MUST also have inside of it whatever - // `validate_publish_request` was doing. - // Also, uncomment when the implementation ready, for now we - // do not need this to block e2e happy test cases. - // session.verify_module_bundle_before_publishing_with_compat_config( - // modules.as_slice(), - // &destination, - // module_storage, - // compat, - // )?; - - // Create module write set for modules to be published. - let write_ops = session - .convert_modules_into_write_ops( - module_storage, - bundle.iter().zip(modules).map(|(m, compiled_module)| { - (m.code().to_vec().into(), compiled_module) - }), - ) - .map_err(|e| e.finish(Location::Undefined))?; - - // Create temporary VM, session, and module storage for running init_module. - // We create a new VM so that type cache is not inconsistent when init_module - // function fails, but loads some new types , type depth formulas, or struct - // name indices. - // TODO(loader_v2): Revisit this to support in a nicer way? - let tmp_module_storage = TemporaryModuleStorage::create( - self.move_vm.runtime_env(), - write_ops, - module_storage, - ); + // Create a temporary storage. If this fails, it means publishing + // is not possible. We use a new VM here so that struct index map + // in the environment, and the type cache inside loader are not + // inconsistent. + // TODO(loader_v2): Revisit this to make it less ugly. let tmp_vm = self.move_vm.clone(); + let tmp_module_storage = TemporaryModuleStorage::new_with_compat_config( + &destination, + // TODO(loader_v2): Make sure to `validate_publish_request` passes to environment + // verifier extension. + tmp_vm.runtime_env(), + compat, + module_storage, + bundle.into_bytes(), + ) + .map_err(|e| e.finish(Location::Undefined))?; + + // Run init_module for each new module. We use a new session for that as well. let mut tmp_session = tmp_vm.new_session( resolver, SessionId::txn_meta(transaction_metadata), @@ -1705,35 +1695,35 @@ impl AptosVM { } let module_id = module.self_id(); - let init_function = tmp_session.load_function( - &tmp_module_storage, - &module_id, - init_func_name, - &[], - ); - if init_function.is_ok() { - if verifier::module_init::verify_module_init_function(module).is_ok() { - tmp_session.execute_function_bypass_visibility( - &module_id, - init_func_name, - vec![], - vec![MoveValue::Signer(destination) - .simple_serialize() - .unwrap()], - gas_meter, - traversal_context, - &tmp_module_storage, - )?; - } else { - // TODO(loader_v2): Use this opportunity to change the error message? - return Err(PartialVMError::new( - StatusCode::CONSTRAINT_NOT_SATISFIED, - ) - .finish(Location::Undefined)); - } + let init_function_exists = tmp_session + .load_function(&tmp_module_storage, &module_id, init_func_name, &[]) + .is_ok(); + if init_function_exists { + verifier::module_init::verify_module_init_function(module) + .map_err(|e| e.finish(Location::Undefined))?; + tmp_session.execute_function_bypass_visibility( + &module_id, + init_func_name, + vec![], + vec![MoveValue::Signer(destination).simple_serialize().unwrap()], + gas_meter, + traversal_context, + &tmp_module_storage, + )?; } } + // Create module write set for modules to be published. We do not care + // here about writes to special addresses because there is no flushing. + let write_ops = session + .convert_modules_into_write_ops( + module_storage, + tmp_module_storage.release_verified_module_bundle(), + ) + .map_err(|e| e.finish(Location::Undefined))?; + let module_write_set = ModuleWriteSet::new(false, write_ops); + + // Process init_module side-effects. let (init_module_changes, empty_write_set) = tmp_session.finish(change_set_configs, module_storage)?; empty_write_set @@ -1743,7 +1733,7 @@ impl AptosVM { .finish(Location::Undefined) })?; - Ok(Some((tmp_module_storage.destroy(), init_module_changes))) + Ok(Some((module_write_set, init_module_changes))) } else { // Validate the module bundle self.validate_publish_request( @@ -1771,13 +1761,11 @@ impl AptosVM { bundle.into_inner(), destination, gas_meter, - module_storage, compat, )?; self.execute_module_initialization( session, - module_storage, gas_meter, modules, exists, diff --git a/aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs b/aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs index 09eedac11c6466..40678718c16b8d 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs @@ -121,10 +121,10 @@ impl<'r, 'l> SessionExt<'r, 'l> { } } - pub fn convert_modules_into_write_ops<'a>( + pub fn convert_modules_into_write_ops( &self, module_storage: &impl AptosModuleStorage, - code_and_modules: impl IntoIterator, + code_and_modules: impl IntoIterator, ) -> PartialVMResult> { let woc = WriteOpConverter::new(self.resolver, self.is_storage_slot_metadata_enabled); woc.convert_modules_into_write_ops(module_storage, code_and_modules) diff --git a/aptos-move/aptos-vm/src/move_vm_ext/write_op_converter.rs b/aptos-move/aptos-vm/src/move_vm_ext/write_op_converter.rs index d2d75dc7411269..df6fab2e1867a1 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/write_op_converter.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/write_op_converter.rs @@ -168,10 +168,10 @@ impl<'r> WriteOpConverter<'r> { /// Converts module bytes and their compiled representation extracted from publish /// request into write ops. Only used by V2 loader implementation. - pub(crate) fn convert_modules_into_write_ops<'a>( + pub(crate) fn convert_modules_into_write_ops( &self, module_storage: &impl AptosModuleStorage, - code_and_modules: impl IntoIterator, + code_and_modules: impl IntoIterator, ) -> PartialVMResult> { let mut write_ops = BTreeMap::new(); for (bytes, compiled_module) in code_and_modules.into_iter() { diff --git a/aptos-move/aptos-vm/src/verifier/module_init.rs b/aptos-move/aptos-vm/src/verifier/module_init.rs index e237c2e1211a03..22ef3f5d730c70 100644 --- a/aptos-move/aptos-vm/src/verifier/module_init.rs +++ b/aptos-move/aptos-vm/src/verifier/module_init.rs @@ -29,7 +29,7 @@ pub fn verify_module_init_function(module: &CompiledModule) -> PartialVMResult<( if fdef.visibility != Visibility::Private { return Err(PartialVMError::new(StatusCode::VERIFICATION_ERROR) - .with_message("module_init_function is not private".to_string())); + .with_message("init_function is not private".to_string())); } let fhandle = module.function_handle_at(fdef.function); @@ -39,7 +39,7 @@ pub fn verify_module_init_function(module: &CompiledModule) -> PartialVMResult<( if !return_.0.is_empty() { return Err(PartialVMError::new(StatusCode::VERIFICATION_ERROR) - .with_message("module_init_function should not return".to_string())); + .with_message("init_function should not return".to_string())); } let non_signer_tokens = parameters @@ -48,7 +48,7 @@ pub fn verify_module_init_function(module: &CompiledModule) -> PartialVMResult<( .any(|e| !is_signer_or_signer_reference(e)); if non_signer_tokens { return Err(PartialVMError::new(StatusCode::VERIFICATION_ERROR) - .with_message("module_init_function should not have no-signer arguments".to_string())); + .with_message("init_function should not have no-signer arguments".to_string())); } Ok(()) } diff --git a/aptos-move/vm-genesis/src/lib.rs b/aptos-move/vm-genesis/src/lib.rs index a5838c10bad02a..131fd0568204ad 100644 --- a/aptos-move/vm-genesis/src/lib.rs +++ b/aptos-move/vm-genesis/src/lib.rs @@ -53,7 +53,10 @@ use move_core_types::{ language_storage::{ModuleId, TypeTag}, value::{serialize_values, MoveTypeLayout, MoveValue}, }; -use move_vm_runtime::module_traversal::{TraversalContext, TraversalStorage}; +use move_vm_runtime::{ + module_traversal::{TraversalContext, TraversalStorage}, + ModuleStorage, TemporaryModuleStorage, +}; use move_vm_types::gas::UnmeteredGasMeter; use once_cell::sync::Lazy; use rand::prelude::*; @@ -181,10 +184,10 @@ pub fn encode_aptos_mainnet_genesis_transaction( new_id[31] = 1; let mut session = vm.new_genesis_session(&resolver, HashValue::new(new_id)); - let module_write_set = publish_framework(&mut session, &module_storage, framework); - let (additional_change_set, empty_module_write_set) = + let loader_v2_module_write_set = publish_framework(&mut session, &module_storage, framework); + let (additional_change_set, loader_v1_module_write_set) = session.finish(&configs, &module_storage).unwrap(); - assert_ok!(empty_module_write_set.is_empty_or_invariant_violation()); + assert!(!loader_v2_module_write_set.is_empty() && !loader_v1_module_write_set.is_empty()); change_set .squash_additional_change_set(additional_change_set) @@ -202,6 +205,11 @@ pub fn encode_aptos_mainnet_genesis_transaction( .any(|(_, op)| op.expect("expect only concrete write ops").is_deletion())); verify_genesis_write_set(change_set.events()); + let module_write_set = if loader_v2_module_write_set.is_empty() { + loader_v1_module_write_set + } else { + loader_v2_module_write_set + }; let change_set = change_set .try_combine_into_storage_change_set(module_write_set) .expect("Constructing a ChangeSet from VMChangeSet should always succeed at genesis"); @@ -316,11 +324,10 @@ pub fn encode_genesis_change_set( new_id[31] = 1; let mut session = vm.new_genesis_session(&resolver, HashValue::new(new_id)); - // Use new flow for publishing straight away. - let module_write_set = publish_framework(&mut session, &module_storage, framework); - let (additional_change_set, empty_module_write_set) = + let loader_v2_module_write_set = publish_framework(&mut session, &module_storage, framework); + let (additional_change_set, loader_v1_module_write_set) = session.finish(&configs, &module_storage).unwrap(); - assert_ok!(empty_module_write_set.is_empty_or_invariant_violation()); + assert!(!loader_v2_module_write_set.is_empty() && !loader_v1_module_write_set.is_empty()); change_set .squash_additional_change_set(additional_change_set) @@ -339,6 +346,11 @@ pub fn encode_genesis_change_set( .any(|(_, op)| op.expect("expect only concrete write ops").is_deletion())); verify_genesis_write_set(change_set.events()); + let module_write_set = if loader_v2_module_write_set.is_empty() { + loader_v1_module_write_set + } else { + loader_v2_module_write_set + }; change_set .try_combine_into_storage_change_set(module_write_set) .expect("Constructing a ChangeSet from VMChangeSet should always succeed at genesis") @@ -382,7 +394,7 @@ fn validate_genesis_config(genesis_config: &GenesisConfiguration) { fn exec_function( session: &mut SessionExt, - module_storage: &impl AptosModuleStorage, + module_storage: &impl ModuleStorage, module_name: &str, function_name: &str, ty_args: Vec, @@ -827,6 +839,28 @@ fn allow_core_resources_to_set_version( ); } +fn initialize_package( + session: &mut SessionExt, + module_storage: &impl ModuleStorage, + addr: AccountAddress, + package: &ReleasePackage, +) { + exec_function( + session, + module_storage, + CODE_MODULE_NAME, + "initialize", + vec![], + vec![ + MoveValue::Signer(CORE_CODE_ADDRESS) + .simple_serialize() + .unwrap(), + MoveValue::Signer(addr).simple_serialize().unwrap(), + bcs::to_bytes(package.package_metadata()).unwrap(), + ], + ); +} + /// Publish the framework release bundle. fn publish_framework( session: &mut SessionExt, @@ -850,51 +884,58 @@ fn publish_package( let modules = pack.sorted_code_and_modules(); let addr = *modules.first().unwrap().1.self_id().address(); - // Add modules to write set. - let package_write_ops = session - .convert_modules_into_write_ops( - module_storage, - modules - .iter() - .map(|(code, compiled_module)| (code.to_vec().into(), compiled_module)), - ) - .unwrap_or_else(|e| { - panic!( - "Failure when creating write ops for package `{}`: {:?}", - pack.package_metadata().name, - e - ) - }); - write_ops.extend(package_write_ops); - - // Ensure all modules are publishable. - let compiled_modules = modules.into_iter().map(|(_, m)| m).collect::>(); - session - .verify_module_bundle_before_publishing(&compiled_modules, &addr, module_storage) - .unwrap_or_else(|e| { - panic!( - "Failure publishing package `{}`: {:?}", - pack.package_metadata().name, - e + let vm = session.get_move_vm(); + if vm.vm_config().use_loader_v2 { + let code = modules + .into_iter() + .map(|(c, _)| c.to_vec().into()) + .collect::>(); + + // State view already contains all code, we just need to: + // 1) check it is publishable, + // 2) convert to write ops. + let tmp_module_storage = + TemporaryModuleStorage::new(&addr, vm.runtime_env(), module_storage, code) + .unwrap_or_else(|e| { + panic!( + "Failure publishing package `{}`: {:?}", + pack.package_metadata().name, + e + ) + }); + initialize_package(session, &tmp_module_storage, addr, pack); + + // Add modules to write set. + let package_write_ops = session + .convert_modules_into_write_ops( + module_storage, + tmp_module_storage.release_verified_module_bundle(), ) - }); - - // Module storage already contains all modules, so we only need to call the initialize - // function with the metadata. - exec_function( - session, - module_storage, - CODE_MODULE_NAME, - "initialize", - vec![], - vec![ - MoveValue::Signer(CORE_CODE_ADDRESS) - .simple_serialize() - .unwrap(), - MoveValue::Signer(addr).simple_serialize().unwrap(), - bcs::to_bytes(pack.package_metadata()).unwrap(), - ], - ); + .unwrap_or_else(|e| { + panic!( + "Failure when creating write ops for package `{}`: {:?}", + pack.package_metadata().name, + e + ) + }); + write_ops.extend(package_write_ops); + } else { + let code = modules + .into_iter() + .map(|(c, _)| c.to_vec()) + .collect::>(); + #[allow(deprecated)] + session + .publish_module_bundle(code, addr, &mut UnmeteredGasMeter) + .unwrap_or_else(|e| { + panic!( + "Failure publishing package `{}`: {:?}", + pack.package_metadata().name, + e + ) + }); + initialize_package(session, module_storage, addr, pack); + } } /// Trigger a reconfiguration. This emits an event that will be passed along to the storage layer. diff --git a/third_party/move/extensions/async/move-async-vm/src/async_vm.rs b/third_party/move/extensions/async/move-async-vm/src/async_vm.rs index 0c5c350686033e..eecfe099f6bfcc 100644 --- a/third_party/move/extensions/async/move-async-vm/src/async_vm.rs +++ b/third_party/move/extensions/async/move-async-vm/src/async_vm.rs @@ -22,7 +22,7 @@ use move_vm_runtime::{ native_extensions::NativeContextExtensions, native_functions::NativeFunction, session::{SerializedReturnValues, Session}, - DummyCodeStorage, + UnreachableCodeStorage, }; use move_vm_test_utils::gas_schedule::{Gas, GasStatus}; use move_vm_types::{ @@ -69,12 +69,19 @@ impl AsyncVM { }) }) .collect(); + let move_vm = MoveVM::new( + natives + .into_iter() + .chain(natives::actor_natives(async_lib_addr, actor_gas_parameters)), + ); + if move_vm.vm_config().use_loader_v2 { + return Err(PartialVMError::new(StatusCode::FEATURE_NOT_ENABLED) + .with_message("AsyncVM does not support loader V2 implementation".to_string()) + .finish(Location::Undefined)); + } + Ok(AsyncVM { - move_vm: MoveVM::new( - natives - .into_iter() - .chain(natives::actor_natives(async_lib_addr, actor_gas_parameters)), - ), + move_vm, actor_metadata, message_table, }) @@ -112,11 +119,16 @@ impl AsyncVM { } } - /// Get the underlying Move VM. - pub fn get_move_vm(&mut self) -> &mut MoveVM { + /// Get the mutable reference to the underlying Move VM. + pub fn get_move_vm_mut(&mut self) -> &mut MoveVM { &mut self.move_vm } + /// Get the reference to the underlying Move VM. + pub fn get_move_vm(&self) -> &MoveVM { + &self.move_vm + } + /// Resolve the message hash into module and handler function. pub fn resolve_message_hash(&self, message_hash: u64) -> Option<&(ModuleId, Identifier)> { self.message_table.get(&message_hash) @@ -183,13 +195,13 @@ impl<'r, 'l> AsyncSession<'r, 'l> { let state_type_tag = TypeTag::Struct(Box::new(actor.state_tag.clone())); let state_type = self .vm_session - .load_type(&state_type_tag, &DummyCodeStorage) + .load_type(&state_type_tag, &UnreachableCodeStorage) .map_err(vm_error_to_async)?; // Check whether the actor state already exists. let state = self .vm_session - .load_resource(&DummyCodeStorage, actor_addr, &state_type) + .load_resource(&UnreachableCodeStorage, actor_addr, &state_type) .map(|(gv, _)| gv) .map_err(partial_vm_error_to_async)?; if state.exists().map_err(partial_vm_error_to_async)? { @@ -212,7 +224,7 @@ impl<'r, 'l> AsyncSession<'r, 'l> { Vec::>::new(), gas_status, &mut TraversalContext::new(&traversal_storage), - &DummyCodeStorage, + &UnreachableCodeStorage, ) .and_then(|ret| Ok((ret, self.vm_session.finish_with_extensions()?))); let gas_used = gas_before.checked_sub(gas_status.remaining_gas()).unwrap(); @@ -280,12 +292,12 @@ impl<'r, 'l> AsyncSession<'r, 'l> { let state_type_tag = TypeTag::Struct(Box::new(actor.state_tag.clone())); let state_type = self .vm_session - .load_type(&state_type_tag, &DummyCodeStorage) + .load_type(&state_type_tag, &UnreachableCodeStorage) .map_err(vm_error_to_async)?; let actor_state_global = self .vm_session - .load_resource(&DummyCodeStorage, actor_addr, &state_type) + .load_resource(&UnreachableCodeStorage, actor_addr, &state_type) .map(|(gv, _)| gv) .map_err(partial_vm_error_to_async)?; let actor_state = actor_state_global @@ -311,7 +323,7 @@ impl<'r, 'l> AsyncSession<'r, 'l> { args, gas_status, &mut TraversalContext::new(&traversal_storage), - &DummyCodeStorage, + &UnreachableCodeStorage, ) .and_then(|ret| Ok((ret, self.vm_session.finish_with_extensions()?))); @@ -360,7 +372,7 @@ impl<'r, 'l> AsyncSession<'r, 'l> { fn to_bcs(&mut self, value: Value, tag: &TypeTag) -> PartialVMResult> { let type_layout = self .vm_session - .get_type_layout(tag, &DummyCodeStorage) + .get_type_layout(tag, &UnreachableCodeStorage) .map_err(|e| e.to_partial())?; value .simple_serialize(&type_layout) diff --git a/third_party/move/extensions/async/move-async-vm/tests/testsuite.rs b/third_party/move/extensions/async/move-async-vm/tests/testsuite.rs index 41d6899c749ce1..2251cffcd1c764 100644 --- a/third_party/move/extensions/async/move-async-vm/tests/testsuite.rs +++ b/third_party/move/extensions/async/move-async-vm/tests/testsuite.rs @@ -2,19 +2,16 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 +use anyhow::bail; use bytes::Bytes; use itertools::Itertools; use move_async_vm::{ actor_metadata, actor_metadata::ActorMetadata, - async_vm::{AsyncResult, AsyncVM, Message}, + async_vm::{AsyncResult, AsyncSession, AsyncVM, Message}, natives::GasParameters as ActorGasParameters, }; -use move_binary_format::{ - access::ModuleAccess, - errors::{PartialVMError, PartialVMResult}, - CompiledModule, -}; +use move_binary_format::{access::ModuleAccess, errors::PartialVMResult}; use move_command_line_common::testing::get_compiler_exp_extension; use move_compiler::{ attr_derivation, compiled_unit::CompiledUnit, diagnostics::report_diagnostics_to_buffer, @@ -28,10 +25,8 @@ use move_core_types::{ language_storage::{ModuleId, StructTag}, metadata::Metadata, value::MoveTypeLayout, - vm_status::StatusCode, }; use move_prover_test_utils::{baseline_test::verify_or_update_baseline, extract_test_directives}; -use move_vm_runtime::{Module, ModuleStorage}; use move_vm_test_utils::gas_schedule::GasStatus; use move_vm_types::resolver::{resource_size, ModuleResolver, ResourceResolver}; use std::{ @@ -39,7 +34,6 @@ use std::{ collections::{BTreeMap, BTreeSet, VecDeque}, path::{Path, PathBuf}, str::FromStr, - sync::Arc, }; const TEST_ADDR: &str = "0x3"; @@ -91,13 +85,18 @@ datatest_stable::harness!(test_runner, "tests/sources", r".*\.move$"); impl Harness { fn run(&self, _module: &str) -> anyhow::Result<()> { + if self.vm.get_move_vm().vm_config().use_loader_v2 { + bail!("AsyncVM does not support loader V2 implementation") + } + let mut gas = GasStatus::new_unmetered(); let mut tick = 0; // Publish modules. let proxy = HarnessProxy { harness: self }; + let mut session = self.vm.new_session(test_account(), 0, &proxy); let mut done = BTreeSet::new(); for id in self.module_cache.keys() { - self.publish_module(id, &mut done, &proxy)?; + self.publish_module(&mut session, id, &mut gas, &mut done)?; } // Initialize actors let mut mailbox: VecDeque = Default::default(); @@ -148,34 +147,27 @@ impl Harness { Ok(()) } - // Note(loader_v2): - // This code reads weird. We do not really publish anything, because module cache - // cache already has all the modules. The session is not also used, so we can just - // ignore the whole thing? Keep like this for now and revisit in the future. fn publish_module( &self, + session: &mut AsyncSession, id: &IdentStr, + gas: &mut GasStatus, done: &mut BTreeSet, - proxy: &HarnessProxy, ) -> anyhow::Result<()> { if done.insert(id.to_owned()) { let cu = self.module_cache.get(id).unwrap(); if let CompiledUnit::Module(m) = cu { for dep in &m.module.module_handles { let dep_id = m.module.identifier_at(dep.name); - self.publish_module(dep_id, done, proxy)? + self.publish_module(session, dep_id, gas, done)? } } self.log(format!("publishing {}", id)); - - let bytes = cu.serialize(None); - let cm = CompiledModule::deserialize(&bytes)?; - - let mut session = self.vm.new_session(test_account(), 0, proxy); + #[allow(deprecated)] session .get_move_session() - .verify_module_bundle_before_publishing(&[cm], &test_account(), proxy)? + .publish_module(cu.serialize(None), test_account(), gas)? } Ok(()) } @@ -410,60 +402,6 @@ impl<'a> ModuleResolver for HarnessProxy<'a> { } } -impl<'a> ModuleStorage for HarnessProxy<'a> { - fn check_module_exists( - &self, - _address: &AccountAddress, - module_name: &IdentStr, - ) -> PartialVMResult { - Ok(self.harness.module_cache.contains_key(module_name)) - } - - fn fetch_module_size_in_bytes( - &self, - _address: &AccountAddress, - module_name: &IdentStr, - ) -> PartialVMResult { - let m = self - .harness - .module_cache - .get(module_name) - .ok_or_else(|| PartialVMError::new(StatusCode::LINKER_ERROR))?; - Ok(m.serialize(None).len()) - } - - fn fetch_module_metadata( - &self, - _address: &AccountAddress, - _module_name: &IdentStr, - ) -> PartialVMResult> { - // Same as ModuleResolver. - Ok(vec![]) - } - - fn fetch_deserialized_module( - &self, - _address: &AccountAddress, - module_name: &IdentStr, - ) -> PartialVMResult> { - let m = self - .harness - .module_cache - .get(module_name) - .ok_or_else(|| PartialVMError::new(StatusCode::LINKER_ERROR))?; - let cm = CompiledModule::deserialize(&m.serialize(None))?; - Ok(Arc::new(cm)) - } - - fn fetch_verified_module( - &self, - _address: &AccountAddress, - _module_name: &IdentStr, - ) -> PartialVMResult> { - unimplemented!() - } -} - impl<'a> ResourceResolver for HarnessProxy<'a> { fn get_resource_bytes_with_metadata_and_layout( &self, diff --git a/third_party/move/move-vm/integration-tests/src/tests/bad_storage_tests.rs b/third_party/move/move-vm/integration-tests/src/tests/bad_storage_tests.rs index a510c3f4ec5427..ffc9a6bd932ff0 100644 --- a/third_party/move/move-vm/integration-tests/src/tests/bad_storage_tests.rs +++ b/third_party/move/move-vm/integration-tests/src/tests/bad_storage_tests.rs @@ -620,6 +620,14 @@ impl ModuleStorage for BogusModuleStorage { Err(PartialVMError::new(self.bad_status_code)) } + fn fetch_module_bytes( + &self, + _address: &AccountAddress, + _module_name: &IdentStr, + ) -> PartialVMResult> { + Err(PartialVMError::new(self.bad_status_code)) + } + fn fetch_module_size_in_bytes( &self, _address: &AccountAddress, diff --git a/third_party/move/move-vm/integration-tests/src/tests/binary_format_version.rs b/third_party/move/move-vm/integration-tests/src/tests/binary_format_version.rs index ab18bb8f46b8be..691d1830b0c300 100644 --- a/third_party/move/move-vm/integration-tests/src/tests/binary_format_version.rs +++ b/third_party/move/move-vm/integration-tests/src/tests/binary_format_version.rs @@ -3,96 +3,128 @@ use move_binary_format::{ deserializer::DeserializerConfig, - file_format::basic_test_script, + file_format::{basic_test_module, basic_test_script}, file_format_common::{IDENTIFIER_SIZE_MAX, VERSION_MAX}, }; use move_core_types::{account_address::AccountAddress, vm_status::StatusCode}; use move_vm_runtime::{ config::VMConfig, module_traversal::*, move_vm::MoveVM, IntoUnsyncCodeStorage, - LocalModuleBytesStorage, + IntoUnsyncModuleStorage, LocalModuleBytesStorage, TemporaryModuleStorage, }; use move_vm_test_utils::InMemoryStorage; use move_vm_types::gas::UnmeteredGasMeter; #[test] fn test_publish_module_with_custom_max_binary_format_version() { - // TODO(loader_v2): Reimplement this test in Aptos codebase to check for deserialization. - - // let m = basic_test_module(); - // let mut b_new = vec![]; - // let mut b_old = vec![]; - // m.serialize_for_version(Some(VERSION_MAX), &mut b_new) - // .unwrap(); - // m.serialize_for_version(Some(VERSION_MAX.checked_sub(1).unwrap()), &mut b_old) - // .unwrap(); - // - // // Should accept both modules with the default settings - // { - // let storage = InMemoryStorage::new(); - // let vm = MoveVM::new(move_stdlib::natives::all_natives( - // AccountAddress::from_hex_literal("0x1").unwrap(), - // move_stdlib::natives::GasParameters::zeros(), - // )); - // let mut sess = vm.new_session(&storage); - // - // #[allow(deprecated)] - // sess.publish_module_bundle( - // vec![b_new.clone()], - // *m.self_id().address(), - // &mut UnmeteredGasMeter, - // &DummyCodeStorage, - // ) - // .unwrap(); - // - // #[allow(deprecated)] - // sess.publish_module_bundle( - // vec![b_old.clone()], - // *m.self_id().address(), - // &mut UnmeteredGasMeter, - // &DummyCodeStorage, - // ) - // .unwrap(); - // } - // - // // Should reject the module with newer version with max binary format version being set to VERSION_MAX - 1 - // { - // let storage = InMemoryStorage::new(); - // let vm = MoveVM::new_with_config( - // move_stdlib::natives::all_natives( - // AccountAddress::from_hex_literal("0x1").unwrap(), - // move_stdlib::natives::GasParameters::zeros(), - // ), - // VMConfig { - // deserializer_config: DeserializerConfig::new( - // VERSION_MAX.checked_sub(1).unwrap(), - // IDENTIFIER_SIZE_MAX, - // ), - // ..Default::default() - // }, - // ); - // let mut sess = vm.new_session(&storage); - // - // #[allow(deprecated)] - // let s = sess - // .publish_module_bundle( - // vec![b_new.clone()], - // *m.self_id().address(), - // &mut UnmeteredGasMeter, - // &DummyCodeStorage, - // ) - // .unwrap_err() - // .major_status(); - // assert_eq!(s, StatusCode::UNKNOWN_VERSION); - // - // #[allow(deprecated)] - // sess.publish_module_bundle( - // vec![b_old.clone()], - // *m.self_id().address(), - // &mut UnmeteredGasMeter, - // &DummyCodeStorage, - // ) - // .unwrap(); - // } + let m = basic_test_module(); + let mut b_new = vec![]; + let mut b_old = vec![]; + m.serialize_for_version(Some(VERSION_MAX), &mut b_new) + .unwrap(); + m.serialize_for_version(Some(VERSION_MAX.checked_sub(1).unwrap()), &mut b_old) + .unwrap(); + + // Should accept both modules with the default settings + { + let vm = MoveVM::new(move_stdlib::natives::all_natives( + AccountAddress::from_hex_literal("0x1").unwrap(), + move_stdlib::natives::GasParameters::zeros(), + )); + + let resource_storage = InMemoryStorage::new(); + let module_storage = + LocalModuleBytesStorage::empty().into_unsync_module_storage(vm.runtime_env()); + + let mut sess = vm.new_session(&resource_storage); + if vm.vm_config().use_loader_v2 { + let module_storage = TemporaryModuleStorage::new( + m.self_addr(), + vm.runtime_env(), + &module_storage, + vec![b_new.clone().into()], + ) + .expect("New module should be publishable"); + TemporaryModuleStorage::new(m.self_addr(), vm.runtime_env(), &module_storage, vec![ + b_old.clone().into(), + ]) + .expect("Old module should be publishable"); + } else { + #[allow(deprecated)] + sess.publish_module_bundle( + vec![b_new.clone()], + *m.self_id().address(), + &mut UnmeteredGasMeter, + ) + .unwrap(); + + #[allow(deprecated)] + sess.publish_module_bundle( + vec![b_old.clone()], + *m.self_id().address(), + &mut UnmeteredGasMeter, + ) + .unwrap(); + } + } + + // Should reject the module with newer version with max binary format version being set to VERSION_MAX - 1 + { + let vm = MoveVM::new_with_config( + move_stdlib::natives::all_natives( + AccountAddress::from_hex_literal("0x1").unwrap(), + move_stdlib::natives::GasParameters::zeros(), + ), + VMConfig { + deserializer_config: DeserializerConfig::new( + VERSION_MAX.checked_sub(1).unwrap(), + IDENTIFIER_SIZE_MAX, + ), + ..Default::default() + }, + ); + + let resource_storage = InMemoryStorage::new(); + let module_storage = + LocalModuleBytesStorage::empty().into_unsync_module_storage(vm.runtime_env()); + + let mut sess = vm.new_session(&resource_storage); + if vm.vm_config().use_loader_v2 { + let result = TemporaryModuleStorage::new( + m.self_addr(), + vm.runtime_env(), + &module_storage, + vec![b_new.clone().into()], + ); + if let Err(err) = result { + assert_eq!(err.major_status(), StatusCode::UNKNOWN_VERSION); + } else { + panic!("Module publishing should fail") + } + TemporaryModuleStorage::new(m.self_addr(), vm.runtime_env(), &module_storage, vec![ + b_old.clone().into(), + ]) + .unwrap(); + } else { + #[allow(deprecated)] + let s = sess + .publish_module_bundle( + vec![b_new.clone()], + *m.self_id().address(), + &mut UnmeteredGasMeter, + ) + .unwrap_err() + .major_status(); + assert_eq!(s, StatusCode::UNKNOWN_VERSION); + + #[allow(deprecated)] + sess.publish_module_bundle( + vec![b_old.clone()], + *m.self_id().address(), + &mut UnmeteredGasMeter, + ) + .unwrap(); + } + } } #[test] diff --git a/third_party/move/move-vm/integration-tests/src/tests/instantiation_tests.rs b/third_party/move/move-vm/integration-tests/src/tests/instantiation_tests.rs index 44e92d7808c85f..27d7c404ac7f78 100644 --- a/third_party/move/move-vm/integration-tests/src/tests/instantiation_tests.rs +++ b/third_party/move/move-vm/integration-tests/src/tests/instantiation_tests.rs @@ -11,11 +11,15 @@ use move_core_types::{ account_address::AccountAddress, ident_str, identifier::Identifier, - language_storage::{StructTag, TypeTag}, + language_storage::{ModuleId, StructTag, TypeTag}, vm_status::StatusCode, }; -use move_vm_runtime::{config::VMConfig, move_vm::MoveVM, DummyCodeStorage}; +use move_vm_runtime::{ + config::VMConfig, move_vm::MoveVM, session::Session, IntoUnsyncCodeStorage, + LocalModuleBytesStorage, ModuleStorage, TemporaryModuleStorage, UnreachableCodeStorage, +}; use move_vm_test_utils::InMemoryStorage; +use move_vm_types::gas::UnmeteredGasMeter; #[test] fn instantiation_err() { @@ -102,8 +106,9 @@ fn instantiation_err() { variant_field_handles: vec![], variant_field_instantiations: vec![], }; - move_bytecode_verifier::verify_module(&cm).expect("verify failed"); + let mut mod_bytes = vec![]; + cm.serialize(&mut mod_bytes).unwrap(); let vm_config = VMConfig { paranoid_type_checks: false, @@ -111,27 +116,11 @@ fn instantiation_err() { }; let vm = MoveVM::new_with_config(vec![], vm_config); - let mut resource_storage: InMemoryStorage = InMemoryStorage::new(); - - // Verify we can publish this module. - { - let mut session = vm.new_session(&resource_storage); - session - .verify_module_bundle_before_publishing( - &[cm.clone()], - cm.self_addr(), - &DummyCodeStorage, - ) - .expect("Module must publish"); - drop(session); - - // Add it to module storage and restart the session. - let mut mod_bytes = vec![]; - cm.serialize(&mut mod_bytes).unwrap(); - resource_storage.publish_or_overwrite_module(cm.self_id(), mod_bytes); - } + let resource_storage: InMemoryStorage = InMemoryStorage::new(); + let module_storage = + LocalModuleBytesStorage::empty().into_unsync_code_storage(vm.runtime_env()); - let mut session = vm.new_session(&resource_storage); + // Prepare type arguments. let mut ty_arg = TypeTag::U128; for _ in 0..4 { ty_arg = TypeTag::Struct(Box::new(StructTag { @@ -142,7 +131,33 @@ fn instantiation_err() { })); } - let res = session.load_function(&DummyCodeStorage, &cm.self_id(), ident_str!("f"), &[ty_arg]); + // Publish (must succeed!) and the load the function. + let mut session = vm.new_session(&resource_storage); + if vm.vm_config().use_loader_v2 { + let module_storage = + TemporaryModuleStorage::new(&addr, vm.runtime_env(), &module_storage, vec![ + mod_bytes.into() + ]) + .expect("Module must publish"); + load_function(&mut session, &module_storage, &cm.self_id(), &[ty_arg]) + } else { + #[allow(deprecated)] + session + .publish_module(mod_bytes, addr, &mut UnmeteredGasMeter) + .expect("Module must publish"); + load_function(&mut session, &UnreachableCodeStorage, &cm.self_id(), &[ + ty_arg, + ]) + } +} + +fn load_function( + session: &mut Session, + module_storage: &impl ModuleStorage, + module_id: &ModuleId, + ty_args: &[TypeTag], +) { + let res = session.load_function(module_storage, module_id, ident_str!("f"), ty_args); assert!( res.is_err(), "Instantiation must fail at load time when converting from type tag to type " diff --git a/third_party/move/move-vm/integration-tests/src/tests/loader_tests.rs b/third_party/move/move-vm/integration-tests/src/tests/loader_tests.rs index 9b21ce27ed59aa..a7aa1e7f363a8c 100644 --- a/third_party/move/move-vm/integration-tests/src/tests/loader_tests.rs +++ b/third_party/move/move-vm/integration-tests/src/tests/loader_tests.rs @@ -19,8 +19,8 @@ use move_core_types::{ language_storage::ModuleId, }; use move_vm_runtime::{ - config::VMConfig, module_traversal::*, move_vm::MoveVM, DummyCodeStorage, - IntoUnsyncModuleStorage, LocalModuleBytesStorage, + config::VMConfig, module_traversal::*, move_vm::MoveVM, IntoUnsyncModuleStorage, + LocalModuleBytesStorage, ModuleStorage, TemporaryModuleStorage, UnreachableCodeStorage, }; use move_vm_test_utils::InMemoryStorage; use move_vm_types::gas::UnmeteredGasMeter; @@ -30,7 +30,6 @@ const WORKING_ACCOUNT: AccountAddress = AccountAddress::TWO; struct Adapter { resource_storage: InMemoryStorage, - module_bytes_storage: LocalModuleBytesStorage, vm: Arc, functions: Vec<(ModuleId, Identifier)>, } @@ -70,7 +69,6 @@ impl Adapter { Self { resource_storage, - module_bytes_storage: LocalModuleBytesStorage::empty(), vm, functions, } @@ -87,59 +85,73 @@ impl Adapter { Self { resource_storage: self.resource_storage, - module_bytes_storage: LocalModuleBytesStorage::empty(), vm, functions: self.functions, } } fn publish_modules(&mut self, modules: Vec) { + let mut session = self.vm.new_session(&self.resource_storage); + for module in modules { let mut binary = vec![]; module .serialize(&mut binary) .unwrap_or_else(|_| panic!("failure in module serialization: {:#?}", module)); - let mut session = self.vm.new_session(&self.resource_storage); + #[allow(deprecated)] session - .verify_module_bundle_before_publishing( - &[module.clone()], - &WORKING_ACCOUNT, - &DummyCodeStorage, - ) - .unwrap_or_else(|_| panic!("failure verifying publishing module: {:#?}", module)); - drop(session); - - self.module_bytes_storage.add_module_bytes( - module.self_addr(), - module.self_name(), - binary.clone().into(), - ); - self.resource_storage - .publish_or_overwrite_module(module.self_id(), binary); + .publish_module(binary, WORKING_ACCOUNT, &mut UnmeteredGasMeter) + .unwrap_or_else(|_| panic!("failure publishing module: {:#?}", module)); } + let changeset = session.finish().expect("failure getting write set"); + self.resource_storage + .apply(changeset) + .expect("failure applying write set"); + } + + fn publish_modules_using_loader_v2<'a, M: ModuleStorage>( + &'a self, + module_storage: &'a M, + modules: Vec, + ) -> TemporaryModuleStorage { + let module_bundle = modules + .into_iter() + .map(|module| { + let mut binary = vec![]; + module + .serialize(&mut binary) + .unwrap_or_else(|_| panic!("failure in module serialization: {:#?}", module)); + binary.into() + }) + .collect(); + TemporaryModuleStorage::new( + &WORKING_ACCOUNT, + self.vm.runtime_env(), + module_storage, + module_bundle, + ) + .expect("failure publishing modules") } fn publish_modules_with_error(&mut self, modules: Vec) { let mut session = self.vm.new_session(&self.resource_storage); + for module in modules { let mut binary = vec![]; module .serialize(&mut binary) .unwrap_or_else(|_| panic!("failure in module serialization: {:#?}", module)); + #[allow(deprecated)] session - .verify_module_bundle_before_publishing( - &[module.clone()], - &WORKING_ACCOUNT, - &DummyCodeStorage, - ) + .publish_module(binary, WORKING_ACCOUNT, &mut UnmeteredGasMeter) .expect_err("publishing must fail"); } } - fn call_functions(&self) { + fn call_functions(&self, module_storage: &impl ModuleStorage) { for (module_id, name) in &self.functions { - self.call_function(module_id, name); + self.call_function(module_id, name, module_storage); } } @@ -148,10 +160,6 @@ impl Adapter { for _ in 0..reps { for (module_id, name) in self.functions.clone() { let resource_storage = self.resource_storage.clone(); - let module_storage = self - .module_bytes_storage - .clone() - .into_unsync_module_storage(self.vm.runtime_env()); scope.spawn(move || { // It is fine to share the VM: we do not publish modules anyway. let mut session = self.vm.as_ref().new_session(&resource_storage); @@ -164,7 +172,7 @@ impl Adapter { Vec::>::new(), &mut UnmeteredGasMeter, &mut TraversalContext::new(&traversal_storage), - &module_storage, + &UnreachableCodeStorage, ) .unwrap_or_else(|e| { panic!("Failure executing {}::{}: {:?}", module_id, name, e) @@ -175,11 +183,12 @@ impl Adapter { }); } - fn call_function(&self, module: &ModuleId, name: &IdentStr) { - let module_storage = self - .module_bytes_storage - .clone() - .into_unsync_module_storage(self.vm.runtime_env()); + fn call_function( + &self, + module: &ModuleId, + name: &IdentStr, + module_storage: &impl ModuleStorage, + ) { let mut session = self.vm.new_session(&self.resource_storage); let traversal_storage = TraversalStorage::new(); session @@ -190,7 +199,7 @@ impl Adapter { Vec::>::new(), &mut UnmeteredGasMeter, &mut TraversalContext::new(&traversal_storage), - &module_storage, + module_storage, ) .unwrap_or_else(|_| panic!("Failure executing {:?}::{:?}", module, name)); } @@ -207,9 +216,17 @@ fn load() { let data_store = InMemoryStorage::new(); let mut adapter = Adapter::new(data_store); let modules = get_modules(); - adapter.publish_modules(modules); + // calls all functions sequentially - adapter.call_functions(); + if adapter.vm.vm_config().use_loader_v2 { + let module_storage = + LocalModuleBytesStorage::empty().into_unsync_module_storage(adapter.vm.runtime_env()); + adapter.publish_modules_using_loader_v2(&module_storage, modules); + adapter.call_functions(&module_storage); + } else { + adapter.publish_modules(modules); + adapter.call_functions(&UnreachableCodeStorage); + } } #[test] @@ -217,9 +234,14 @@ fn load_concurrent() { let data_store = InMemoryStorage::new(); let mut adapter = Adapter::new(data_store); let modules = get_modules(); - adapter.publish_modules(modules); - // makes 15 threads - adapter.call_functions_async(3); + + // Makes 15 threads. Test loader V1 here only because we do not have Sync + // module storage implementation here. Also, even loader V1 tests are not + // really useful because VM cannot be shared across threads... + if !adapter.vm.vm_config().use_loader_v2 { + adapter.publish_modules(modules); + adapter.call_functions_async(3); + } } #[test] @@ -227,17 +249,20 @@ fn load_concurrent_many() { let data_store = InMemoryStorage::new(); let mut adapter = Adapter::new(data_store); let modules = get_modules(); - adapter.publish_modules(modules); - // makes 150 threads - adapter.call_functions_async(30); + + // Makes 150 threads. Test loader V1 here only because we do not have Sync + // module storage implementation here. Also, even loader V1 tests are not + // really useful because VM cannot be shared across threads... + if !adapter.vm.vm_config().use_loader_v2 { + adapter.publish_modules(modules); + adapter.call_functions_async(30); + } } #[test] fn load_phantom_module() { let data_store = InMemoryStorage::new(); let mut adapter = Adapter::new(data_store); - let modules = get_modules(); - adapter.publish_modules(modules); let mut module = empty_module(); module.address_identifiers[0] = WORKING_ACCOUNT; @@ -278,19 +303,21 @@ fn load_phantom_module() { }), }); + let mut modules = get_modules(); let module_id = module.self_id(); - adapter.publish_modules(vec![module]); + modules.push(module); if adapter.vm.vm_config().use_loader_v2 { + let module_storage = + LocalModuleBytesStorage::empty().into_unsync_module_storage(adapter.vm.runtime_env()); + let new_module_storage = adapter.publish_modules_using_loader_v2(&module_storage, modules); + let mut session = adapter.vm.new_session(&adapter.resource_storage); - let module_storage = adapter - .module_bytes_storage - .clone() - .into_unsync_module_storage(adapter.vm.runtime_env()); let _ = session - .load_function(&module_storage, &module_id, ident_str!("foo"), &[]) + .load_function(&new_module_storage, &module_id, ident_str!("foo"), &[]) .unwrap(); } else { + adapter.publish_modules(modules); #[allow(deprecated)] adapter .vm @@ -303,8 +330,6 @@ fn load_phantom_module() { fn load_with_extra_ability() { let data_store = InMemoryStorage::new(); let mut adapter = Adapter::new(data_store); - let modules = get_modules(); - adapter.publish_modules(modules); let mut module = empty_module(); module.address_identifiers[0] = WORKING_ACCOUNT; @@ -348,19 +373,21 @@ fn load_with_extra_ability() { }), }); + let mut modules = get_modules(); let module_id = module.self_id(); - adapter.publish_modules(vec![module]); + modules.push(module); if adapter.vm.vm_config().use_loader_v2 { + let module_storage = + LocalModuleBytesStorage::empty().into_unsync_module_storage(adapter.vm.runtime_env()); + let new_module_storage = adapter.publish_modules_using_loader_v2(&module_storage, modules); + let mut session = adapter.vm.new_session(&adapter.resource_storage); - let module_storage = adapter - .module_bytes_storage - .clone() - .into_unsync_module_storage(adapter.vm.runtime_env()); let _ = session - .load_function(&module_storage, &module_id, ident_str!("foo"), &[]) + .load_function(&new_module_storage, &module_id, ident_str!("foo"), &[]) .unwrap(); } else { + adapter.publish_modules(modules); #[allow(deprecated)] adapter .vm diff --git a/third_party/move/move-vm/integration-tests/src/tests/native_tests.rs b/third_party/move/move-vm/integration-tests/src/tests/native_tests.rs index 5144e44d94324f..1c2f6c9c5d2821 100644 --- a/third_party/move/move-vm/integration-tests/src/tests/native_tests.rs +++ b/third_party/move/move-vm/integration-tests/src/tests/native_tests.rs @@ -6,10 +6,12 @@ use move_binary_format::errors::PartialVMResult; use move_bytecode_verifier::VerifierConfig; use move_core_types::{ account_address::AccountAddress, gas_algebra::InternalGas, identifier::Identifier, + language_storage::ModuleId, }; use move_vm_runtime::{ config::VMConfig, module_traversal::*, move_vm::MoveVM, native_functions::NativeFunction, - DummyCodeStorage, + IntoUnsyncCodeStorage, LocalModuleBytesStorage, ModuleStorage, TemporaryModuleStorage, + UnreachableCodeStorage, }; use move_vm_test_utils::InMemoryStorage; use move_vm_types::{gas::UnmeteredGasMeter, natives::function::NativeResult}; @@ -28,7 +30,6 @@ fn make_failed_native() -> NativeFunction { #[test] fn test_publish_module_with_nested_loops() { - // Compile modules and scripts. let code = r#" module {{ADDR}}::M { entry fun foo() { @@ -54,7 +55,6 @@ fn test_publish_module_with_nested_loops() { m.serialize(&mut m_blob).unwrap(); let traversal_storage = TraversalStorage::new(); - // Should succeed with max_loop_depth = 2 { let natives = vec![( TEST_ADDR, @@ -70,58 +70,84 @@ fn test_publish_module_with_nested_loops() { ..Default::default() }); - let mut resource_storage = InMemoryStorage::new(); - { - let mut sess = vm.new_session(&resource_storage); - sess.verify_module_bundle_before_publishing( - &[m.clone()], - &TEST_ADDR, - &DummyCodeStorage, - ) - .unwrap(); - drop(sess); - resource_storage.publish_or_overwrite_module(m.self_id(), m_blob.clone()); - } + let resource_storage = InMemoryStorage::new(); + let module_storage = + LocalModuleBytesStorage::empty().into_unsync_code_storage(vm.runtime_env()); let mut sess = vm.new_session(&resource_storage); - let func = sess - .load_function( - &DummyCodeStorage, + if vm.vm_config().use_loader_v2 { + let module_storage = + TemporaryModuleStorage::new(&TEST_ADDR, vm.runtime_env(), &module_storage, vec![ + m_blob.clone().into(), + ]) + .expect("Module should be publishable"); + load_and_run_functions( + &vm, + &resource_storage, + &module_storage, + &traversal_storage, + &m.self_id(), + ); + } else { + #[allow(deprecated)] + sess.publish_module(m_blob.clone(), TEST_ADDR, &mut UnmeteredGasMeter) + .unwrap(); + load_and_run_functions( + &vm, + &resource_storage, + &UnreachableCodeStorage, + &traversal_storage, &m.self_id(), - &Identifier::new("foo").unwrap(), - &[], - ) - .unwrap(); - let err1 = sess - .execute_entry_function( - func, - Vec::>::new(), - &mut UnmeteredGasMeter, - &mut TraversalContext::new(&traversal_storage), - &DummyCodeStorage, - ) - .unwrap_err(); + ); + }; + } +} - assert!(err1.exec_state().unwrap().stack_trace().is_empty()); +fn load_and_run_functions( + vm: &MoveVM, + resource_storage: &InMemoryStorage, + module_storage: &impl ModuleStorage, + traversal_storage: &TraversalStorage, + module_id: &ModuleId, +) { + let mut sess = vm.new_session(resource_storage); + let func = sess + .load_function( + module_storage, + module_id, + &Identifier::new("foo").unwrap(), + &[], + ) + .unwrap(); + let err1 = sess + .execute_entry_function( + func, + Vec::>::new(), + &mut UnmeteredGasMeter, + &mut TraversalContext::new(traversal_storage), + module_storage, + ) + .unwrap_err(); - let func = sess - .load_function( - &DummyCodeStorage, - &m.self_id(), - &Identifier::new("foo2").unwrap(), - &[], - ) - .unwrap(); - let err2 = sess - .execute_entry_function( - func, - Vec::>::new(), - &mut UnmeteredGasMeter, - &mut TraversalContext::new(&traversal_storage), - &DummyCodeStorage, - ) - .unwrap_err(); + assert!(err1.exec_state().unwrap().stack_trace().is_empty()); - assert_eq!(err2.exec_state().unwrap().stack_trace().len(), 1); - } + let func = sess + .load_function( + module_storage, + module_id, + &Identifier::new("foo2").unwrap(), + &[], + ) + .unwrap(); + let err2 = sess + .execute_entry_function( + func, + Vec::>::new(), + &mut UnmeteredGasMeter, + &mut TraversalContext::new(traversal_storage), + module_storage, + ) + .unwrap_err(); + + assert_eq!(err2.exec_state().unwrap().stack_trace().len(), 1); } diff --git a/third_party/move/move-vm/integration-tests/src/tests/nested_loop_tests.rs b/third_party/move/move-vm/integration-tests/src/tests/nested_loop_tests.rs index cf69a371d4a656..0ae0778150f324 100644 --- a/third_party/move/move-vm/integration-tests/src/tests/nested_loop_tests.rs +++ b/third_party/move/move-vm/integration-tests/src/tests/nested_loop_tests.rs @@ -6,7 +6,7 @@ use move_bytecode_verifier::VerifierConfig; use move_core_types::account_address::AccountAddress; use move_vm_runtime::{ config::VMConfig, module_traversal::*, move_vm::MoveVM, IntoUnsyncCodeStorage, - IntoUnsyncModuleStorage, LocalModuleBytesStorage, + IntoUnsyncModuleStorage, LocalModuleBytesStorage, TemporaryModuleStorage, }; use move_vm_test_utils::InMemoryStorage; use move_vm_types::gas::UnmeteredGasMeter; @@ -15,8 +15,6 @@ const TEST_ADDR: AccountAddress = AccountAddress::new([42; AccountAddress::LENGT #[test] fn test_publish_module_with_nested_loops() { - // Compile the modules and scripts. - // TODO: find a better way to include the Signer module. let code = r#" module {{ADDR}}::M { fun foo() { @@ -59,8 +57,17 @@ fn test_publish_module_with_nested_loops() { LocalModuleBytesStorage::empty().into_unsync_module_storage(vm.runtime_env()); let mut sess = vm.new_session(&resource_storage); - sess.verify_module_bundle_before_publishing(&[m.clone()], &TEST_ADDR, &module_storage) - .unwrap(); + if vm.vm_config().use_loader_v2 { + let result = + TemporaryModuleStorage::new(&TEST_ADDR, vm.runtime_env(), &module_storage, vec![ + m_blob.clone().into(), + ]); + assert!(result.is_ok()); + } else { + #[allow(deprecated)] + sess.publish_module(m_blob.clone(), TEST_ADDR, &mut UnmeteredGasMeter) + .unwrap(); + } } // Should fail with max_loop_depth = 1 @@ -84,15 +91,22 @@ fn test_publish_module_with_nested_loops() { LocalModuleBytesStorage::empty().into_unsync_module_storage(vm.runtime_env()); let mut sess = vm.new_session(&resource_storage); - sess.verify_module_bundle_before_publishing(&[m], &TEST_ADDR, &module_storage) - .unwrap_err(); + if vm.vm_config().use_loader_v2 { + let result = + TemporaryModuleStorage::new(&TEST_ADDR, vm.runtime_env(), &module_storage, vec![ + m_blob.clone().into(), + ]); + assert!(result.is_err()); + } else { + #[allow(deprecated)] + sess.publish_module(m_blob.clone(), TEST_ADDR, &mut UnmeteredGasMeter) + .unwrap_err(); + } } } #[test] fn test_run_script_with_nested_loops() { - // Compile the modules and scripts. - // TODO: find a better way to include the Signer module. let code = r#" script { fun main() { diff --git a/third_party/move/move-vm/runtime/src/lib.rs b/third_party/move/move-vm/runtime/src/lib.rs index a042690820c5f8..4ecc912f561a20 100644 --- a/third_party/move/move-vm/runtime/src/lib.rs +++ b/third_party/move/move-vm/runtime/src/lib.rs @@ -37,12 +37,13 @@ pub use storage::{ dummy::{use_loader_v2_based_on_env, DummyCodeStorage}, environment::RuntimeEnvironment, implementations::{ + unreachable_code_storage::UnreachableCodeStorage, unsync_code_storage::{IntoUnsyncCodeStorage, UnsyncCodeStorage}, unsync_module_storage::{ - IntoUnsyncModuleStorage, LocalModuleBytesStorage, ModuleBytesStorage, - UnsyncModuleStorage, + IntoUnsyncModuleStorage, LocalModuleBytesStorage, UnsyncModuleStorage, }, }, - module_storage::ModuleStorage, + module_storage::{ModuleBytesStorage, ModuleStorage}, + publishing::TemporaryModuleStorage, script_storage::{script_hash, ScriptStorage}, }; diff --git a/third_party/move/move-vm/runtime/src/loader/mod.rs b/third_party/move/move-vm/runtime/src/loader/mod.rs index 35ff79fd3e0034..5420b2b9d153ae 100644 --- a/third_party/move/move-vm/runtime/src/loader/mod.rs +++ b/third_party/move/move-vm/runtime/src/loader/mod.rs @@ -5,8 +5,7 @@ use crate::{ config::VMConfig, data_cache::TransactionDataCache, logging::expect_no_verification_errors, module_traversal::TraversalContext, script_hash, - storage::module_storage::ModuleStorage as ModuleStorageV2, unexpected_unimplemented_error, - RuntimeEnvironment, + storage::module_storage::ModuleStorage as ModuleStorageV2, RuntimeEnvironment, }; use hashbrown::Equivalent; use lazy_static::lazy_static; @@ -273,25 +272,6 @@ impl Loader { } } - pub(crate) fn load_module( - &self, - id: &ModuleId, - data_store: &mut TransactionDataCache, - module_store: &ModuleStorageAdapter, - module_storage: &impl ModuleStorageV2, - ) -> VMResult> { - match self { - Loader::V1(loader) => loader.load_module(id, data_store, module_store), - Loader::V2(loader) => loader - .load_module(module_storage, id.address(), id.name()) - .map_err(|e| { - // TODO(loader_v2): Revisit errors... - let e = e.finish(Location::Undefined); - expect_no_verification_errors(e) - }), - } - } - fn load_function_without_type_args( &self, module_id: &ModuleId, @@ -323,23 +303,6 @@ impl Loader { } } - pub(crate) fn verify_module_bundle_for_publication( - &self, - modules: &[CompiledModule], - data_store: &mut TransactionDataCache, - module_store: &ModuleStorageAdapter, - _module_storage: &impl ModuleStorageV2, - ) -> VMResult<()> { - match self { - Self::V1(loader) => { - loader.verify_module_bundle_for_publication(modules, data_store, module_store) - }, - Self::V2(_loader) => { - unexpected_unimplemented_error!().map_err(|e| e.finish(Location::Undefined)) - }, - } - } - // // Helpers for loading and verification // diff --git a/third_party/move/move-vm/runtime/src/runtime.rs b/third_party/move/move-vm/runtime/src/runtime.rs index 9038e84de4b88a..4d91086a5c2827 100644 --- a/third_party/move/move-vm/runtime/src/runtime.rs +++ b/third_party/move/move-vm/runtime/src/runtime.rs @@ -61,20 +61,58 @@ impl VMRuntime { } } - pub(crate) fn verify_module_bundle_before_publishing( + #[deprecated] + pub(crate) fn publish_module_bundle( &self, - compiled_modules: &[CompiledModule], - sender: &AccountAddress, + modules: Vec>, + sender: AccountAddress, data_store: &mut TransactionDataCache, module_store: &ModuleStorageAdapter, + _gas_meter: &mut impl GasMeter, compat: Compatibility, - module_storage: &impl ModuleStorageV2, ) -> VMResult<()> { + // We must be using V1 loader flow here. + let loader = match &self.loader { + Loader::V1(loader) => loader, + Loader::V2(_) => { + let msg = + "Loader V2 cannot be used in V1 runtime::publish_module_bundle".to_string(); + return Err( + PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) + .with_message(msg) + .finish(Location::Undefined), + ); + }, + }; + + // deserialize the modules. Perform bounds check. After this indexes can be + // used with the `[]` operator + let compiled_modules = match modules + .iter() + .map(|blob| { + CompiledModule::deserialize_with_config( + blob, + &loader.vm_config().deserializer_config, + ) + }) + .collect::>>() + { + Ok(modules) => modules, + Err(err) => { + return Err(err + .append_message_with_separator( + '\n', + "[VM] module deserialization failed".to_string(), + ) + .finish(Location::Undefined)); + }, + }; + // Make sure all modules' self addresses matches the transaction sender. The self address is // where the module will actually be published. If we did not check this, the sender could // publish a module under anyone's account. - for module in compiled_modules { - if module.address() != sender { + for module in &compiled_modules { + if module.address() != &sender { return Err(verification_error( StatusCode::MODULE_ADDRESS_DOES_NOT_MATCH_SENDER, IndexKind::AddressIdentifier, @@ -92,29 +130,14 @@ impl VMRuntime { // // TODO: in the future, we may want to add restrictions on module republishing, possibly by // changing the bytecode format to include an `is_upgradable` flag in the CompiledModule. - for module in compiled_modules { + for module in &compiled_modules { let module_id = module.self_id(); - let module_exists = match self.loader() { - Loader::V1(_) => - { - #[allow(deprecated)] - data_store.exists_module(&module_id)? - }, - Loader::V2(_) => module_storage - .check_module_exists(module.self_addr(), module.self_name()) - .map_err(|e| e.finish(Location::Undefined))?, - }; - - if module_exists && compat.need_check_compat() { - let old_module_ref = self.loader.load_module( - &module_id, - data_store, - module_store, - module_storage, - )?; + #[allow(deprecated)] + if data_store.exists_module(&module_id)? && compat.need_check_compat() { + let old_module_ref = loader.load_module(&module_id, data_store, module_store)?; let old_module = old_module_ref.module(); - if self.loader.vm_config().use_compatibility_checker_v2 { + if loader.vm_config().use_compatibility_checker_v2 { compat .check(old_module, module) .map_err(|e| e.finish(Location::Undefined))? @@ -137,57 +160,7 @@ impl VMRuntime { } // Perform bytecode and loading verification. Modules must be sorted in topological order. - self.loader.verify_module_bundle_for_publication( - compiled_modules, - data_store, - module_store, - module_storage, - )?; - - Ok(()) - } - - pub(crate) fn publish_module_bundle( - &self, - modules: Vec>, - sender: AccountAddress, - data_store: &mut TransactionDataCache, - module_store: &ModuleStorageAdapter, - _gas_meter: &mut impl GasMeter, - compat: Compatibility, - module_storage: &impl ModuleStorageV2, - ) -> VMResult<()> { - // deserialize the modules. Perform bounds check. After this indexes can be - // used with the `[]` operator - let compiled_modules = match modules - .iter() - .map(|blob| { - CompiledModule::deserialize_with_config( - blob, - &self.loader.vm_config().deserializer_config, - ) - }) - .collect::>>() - { - Ok(modules) => modules, - Err(err) => { - return Err(err - .append_message_with_separator( - '\n', - "[VM] module deserialization failed".to_string(), - ) - .finish(Location::Undefined)); - }, - }; - - self.verify_module_bundle_before_publishing( - &compiled_modules, - &sender, - data_store, - module_store, - compat, - module_storage, - )?; + loader.verify_module_bundle_for_publication(&compiled_modules, data_store, module_store)?; // NOTE: we want to (informally) argue that all modules pass the linking check before being // published to the data store. @@ -242,18 +215,7 @@ impl VMRuntime { // all the checks, then the whole bundle can be published/upgraded together. Otherwise, // none of the module can be published/updated. - // Sanity check: we should only be calling the verification API with V2 loader design. - if let Loader::V2(_) = self.loader() { - return Err( - PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) - .with_message( - "Can never publish modules via V2 workflow to data cache".to_string(), - ) - .finish(Location::Undefined), - ); - } - - // All modules verified, publish them to data cache. + // All modules verified, publish them to data cache for (module, blob) in compiled_modules.into_iter().zip(modules.into_iter()) { #[allow(deprecated)] let is_republishing = data_store.exists_module(&module.self_id())?; diff --git a/third_party/move/move-vm/runtime/src/session.rs b/third_party/move/move-vm/runtime/src/session.rs index 87d21197c3dbdf..276c2faf3bc7f1 100644 --- a/third_party/move/move-vm/runtime/src/session.rs +++ b/third_party/move/move-vm/runtime/src/session.rs @@ -13,9 +13,7 @@ use crate::{ ScriptStorage, }; use bytes::Bytes; -use move_binary_format::{ - compatibility::Compatibility, errors::*, file_format::LocalIndex, CompiledModule, -}; +use move_binary_format::{compatibility::Compatibility, errors::*, file_format::LocalIndex}; use move_core_types::{ account_address::AccountAddress, effects::{ChangeSet, Changes}, @@ -182,11 +180,7 @@ impl<'r, 'l> Session<'r, 'l> { ) } - /// Publish a series of modules. - /// - /// The Move VM MUST return a user error, i.e., an error that's not an invariant violation, if - /// any module fails to deserialize or verify. The publishing of the module series is an - /// all-or-nothing action: either all modules are published to the data store or none is. + /// Publish the given module. /// /// The Move VM MUST return a user error, i.e., an error that's not an invariant violation, if /// - The module fails to deserialize or verify. @@ -194,6 +188,29 @@ impl<'r, 'l> Session<'r, 'l> { /// - (Republishing-only) the module to be updated is not backward compatible with the old module. /// - (Republishing-only) the module to be updated introduces cyclic dependencies. /// + /// The Move VM should not be able to produce other user errors. + /// Besides, no user input should cause the Move VM to return an invariant violation. + /// + /// In case an invariant violation occurs, the whole Session should be considered corrupted and + /// one shall not proceed with effect generation. + #[deprecated] + pub fn publish_module( + &mut self, + module: Vec, + sender: AccountAddress, + gas_meter: &mut impl GasMeter, + ) -> VMResult<()> { + #[allow(deprecated)] + self.publish_module_bundle(vec![module], sender, gas_meter) + } + + /// Publish a series of modules. + /// + /// The Move VM MUST return a user error, i.e., an error that's not an invariant violation, if + /// any module fails to deserialize or verify (see the full list of failing conditions in the + /// `publish_module` API). The publishing of the module series is an all-or-nothing action: + /// either all modules are published to the data store or none is. + /// /// Similar to the `publish_module` API, the Move VM should not be able to produce other user /// errors. Besides, no user input should cause the Move VM to return an invariant violation. /// @@ -203,53 +220,57 @@ impl<'r, 'l> Session<'r, 'l> { /// This operation performs compatibility checks if a module is replaced. See also /// `move_binary_format::compatibility`. #[deprecated] - pub fn publish_module_bundle_with_compat_config( + pub fn publish_module_bundle( &mut self, modules: Vec>, sender: AccountAddress, gas_meter: &mut impl GasMeter, - module_storage: &impl ModuleStorage, - compat_config: Compatibility, ) -> VMResult<()> { + #[allow(deprecated)] self.move_vm.runtime.publish_module_bundle( modules, sender, &mut self.data_cache, &self.module_store, gas_meter, - compat_config, - module_storage, + Compatibility::full_check(), ) } - pub fn verify_module_bundle_before_publishing( + /// Same like `publish_module_bundle` but with a custom compatibility check. + #[deprecated] + pub fn publish_module_bundle_with_compat_config( &mut self, - modules: &[CompiledModule], - sender: &AccountAddress, - module_storage: &impl ModuleStorage, + modules: Vec>, + sender: AccountAddress, + gas_meter: &mut impl GasMeter, + compat_config: Compatibility, ) -> VMResult<()> { - self.verify_module_bundle_before_publishing_with_compat_config( + #[allow(deprecated)] + self.move_vm.runtime.publish_module_bundle( modules, sender, - module_storage, - Compatibility::full_check(), + &mut self.data_cache, + &self.module_store, + gas_meter, + compat_config, ) } - pub fn verify_module_bundle_before_publishing_with_compat_config( + pub fn publish_module_bundle_relax_compatibility( &mut self, - modules: &[CompiledModule], - sender: &AccountAddress, - module_storage: &impl ModuleStorage, - compat_config: Compatibility, + modules: Vec>, + sender: AccountAddress, + gas_meter: &mut impl GasMeter, ) -> VMResult<()> { - self.move_vm.runtime.verify_module_bundle_before_publishing( + #[allow(deprecated)] + self.move_vm.runtime.publish_module_bundle( modules, sender, &mut self.data_cache, &self.module_store, - compat_config, - module_storage, + gas_meter, + Compatibility::no_check(), ) } diff --git a/third_party/move/move-vm/runtime/src/storage/dummy.rs b/third_party/move/move-vm/runtime/src/storage/dummy.rs index 974dee6e4b9717..67aed8e0852c92 100644 --- a/third_party/move/move-vm/runtime/src/storage/dummy.rs +++ b/third_party/move/move-vm/runtime/src/storage/dummy.rs @@ -5,6 +5,7 @@ use crate::{ loader::{Module, Script}, storage::{module_storage::ModuleStorage, script_storage::ScriptStorage}, }; +use bytes::Bytes; use move_binary_format::{ errors::{PartialVMError, PartialVMResult}, file_format::CompiledScript, @@ -55,6 +56,14 @@ impl ModuleStorage for DummyCodeStorage { unexpected_unimplemented_error!() } + fn fetch_module_bytes( + &self, + _address: &AccountAddress, + _module_name: &IdentStr, + ) -> PartialVMResult> { + unexpected_unimplemented_error!() + } + fn fetch_module_metadata( &self, _address: &AccountAddress, diff --git a/third_party/move/move-vm/runtime/src/storage/implementations/mod.rs b/third_party/move/move-vm/runtime/src/storage/implementations/mod.rs index 5651d91bd4ba8c..3e801a8a596b48 100644 --- a/third_party/move/move-vm/runtime/src/storage/implementations/mod.rs +++ b/third_party/move/move-vm/runtime/src/storage/implementations/mod.rs @@ -2,5 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 pub mod errors; +pub mod unreachable_code_storage; pub mod unsync_code_storage; pub mod unsync_module_storage; diff --git a/third_party/move/move-vm/runtime/src/storage/implementations/unreachable_code_storage.rs b/third_party/move/move-vm/runtime/src/storage/implementations/unreachable_code_storage.rs new file mode 100644 index 00000000000000..57b0f4666def72 --- /dev/null +++ b/third_party/move/move-vm/runtime/src/storage/implementations/unreachable_code_storage.rs @@ -0,0 +1,99 @@ +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + +use crate::{Module, ModuleStorage, Script, ScriptStorage}; +use bytes::Bytes; +use move_binary_format::{ + errors::{PartialVMError, PartialVMResult}, + file_format::CompiledScript, + CompiledModule, +}; +use move_core_types::{ + account_address::AccountAddress, identifier::IdentStr, metadata::Metadata, + vm_status::StatusCode, +}; +use std::sync::Arc; + +/// An error which is returned in case unreachable code is reached. This is just a safety +/// precaution to avoid panics in case we forget some gating. +#[macro_export] +macro_rules! unreachable_error { + () => { + Err( + PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR).with_message( + "Loader V1 implementation should never use module or script storage".to_string(), + ), + ) + }; +} + +/// Implementation of code storage (for modules and scripts) traits, to be used in case VM +/// is using V1 loader implementation. For example [Session::execute_entry_function] has +/// to take a reference to loader V2 storage interfaces, even if VM uses V1 loader. In this +/// case, they would be just unreachable. +/// Used as a placeholder so that existing APIs can work, i.e., for now client side which has no +/// script or module storage can be still connected to the new V2 APIs by using the dummy. +pub struct UnreachableCodeStorage; + +impl ModuleStorage for UnreachableCodeStorage { + fn check_module_exists( + &self, + _address: &AccountAddress, + _module_name: &IdentStr, + ) -> PartialVMResult { + unreachable_error!() + } + + fn fetch_module_bytes( + &self, + _address: &AccountAddress, + _module_name: &IdentStr, + ) -> PartialVMResult> { + unreachable_error!() + } + + fn fetch_module_size_in_bytes( + &self, + _address: &AccountAddress, + _module_name: &IdentStr, + ) -> PartialVMResult { + unreachable_error!() + } + + fn fetch_module_metadata( + &self, + _address: &AccountAddress, + _module_name: &IdentStr, + ) -> PartialVMResult> { + unreachable_error!() + } + + fn fetch_deserialized_module( + &self, + _address: &AccountAddress, + _module_name: &IdentStr, + ) -> PartialVMResult> { + unreachable_error!() + } + + fn fetch_verified_module( + &self, + _address: &AccountAddress, + _module_name: &IdentStr, + ) -> PartialVMResult> { + unreachable_error!() + } +} + +impl ScriptStorage for UnreachableCodeStorage { + fn fetch_deserialized_script( + &self, + _serialized_script: &[u8], + ) -> PartialVMResult> { + unreachable_error!() + } + + fn fetch_verified_script(&self, _serialized_script: &[u8]) -> PartialVMResult> { + unreachable_error!() + } +} diff --git a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs index c5996ab7022d72..a31cd34f9fe9d0 100644 --- a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs +++ b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_code_storage.rs @@ -6,10 +6,11 @@ use crate::{ storage::{ environment::WithEnvironment, implementations::unsync_module_storage::IntoUnsyncModuleStorage, + module_storage::ModuleBytesStorage, }, - Module, ModuleBytesStorage, ModuleStorage, RuntimeEnvironment, Script, ScriptStorage, - UnsyncModuleStorage, + Module, ModuleStorage, RuntimeEnvironment, Script, ScriptStorage, UnsyncModuleStorage, }; +use bytes::Bytes; use move_binary_format::{ access::ScriptAccess, errors::{PartialVMError, PartialVMResult}, @@ -177,6 +178,14 @@ impl ModuleStorage for UnsyncCodeStorage .check_module_exists(address, module_name) } + fn fetch_module_bytes( + &self, + address: &AccountAddress, + module_name: &IdentStr, + ) -> PartialVMResult> { + self.module_storage.fetch_module_bytes(address, module_name) + } + fn fetch_module_size_in_bytes( &self, address: &AccountAddress, diff --git a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_module_storage.rs b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_module_storage.rs index 049699a7501c65..b7388a3b3a2654 100644 --- a/third_party/move/move-vm/runtime/src/storage/implementations/unsync_module_storage.rs +++ b/third_party/move/move-vm/runtime/src/storage/implementations/unsync_module_storage.rs @@ -5,7 +5,7 @@ use crate::{ module_cyclic_dependency_error, module_linker_error, storage::{ environment::{RuntimeEnvironment, WithEnvironment}, - module_storage::ModuleStorage, + module_storage::{ModuleBytesStorage, ModuleStorage}, }, Module, }; @@ -25,16 +25,6 @@ use std::{ sync::Arc, }; -/// Storage that contains serialized modules. Clients can implement this trait -/// for their own backends, so that [UnsyncModuleStorage] can use them. -pub trait ModuleBytesStorage { - fn fetch_module_bytes( - &self, - address: &AccountAddress, - module_name: &IdentStr, - ) -> PartialVMResult>; -} - /// Represents an in-memory storage that contains modules' bytes. #[derive(Clone)] pub struct LocalModuleBytesStorage(BTreeMap>); @@ -120,8 +110,7 @@ impl<'e, B: ModuleBytesStorage> UnsyncModuleStorage<'e, B> { address: &AccountAddress, module_name: &IdentStr, ) -> PartialVMResult { - self.byte_storage - .fetch_module_bytes(address, module_name)? + self.fetch_module_bytes(address, module_name)? .ok_or_else(|| module_linker_error!(address, module_name)) } @@ -269,6 +258,11 @@ impl<'e, B: ModuleBytesStorage> UnsyncModuleStorage<'e, B> { pub fn byte_storage(&self) -> &B { &self.byte_storage } + + /// Returns the byte storage used by this module storage. + pub(crate) fn release_byte_storage(self) -> B { + self.byte_storage + } } impl<'e, B: ModuleBytesStorage> WithEnvironment for UnsyncModuleStorage<'e, B> { @@ -291,6 +285,14 @@ impl<'e, B: ModuleBytesStorage> ModuleStorage for UnsyncModuleStorage<'e, B> { .is_some()) } + fn fetch_module_bytes( + &self, + address: &AccountAddress, + module_name: &IdentStr, + ) -> PartialVMResult> { + self.byte_storage.fetch_module_bytes(address, module_name) + } + fn fetch_module_size_in_bytes( &self, address: &AccountAddress, diff --git a/third_party/move/move-vm/runtime/src/storage/mod.rs b/third_party/move/move-vm/runtime/src/storage/mod.rs index 12e2628eb9af22..0ea549f471458b 100644 --- a/third_party/move/move-vm/runtime/src/storage/mod.rs +++ b/third_party/move/move-vm/runtime/src/storage/mod.rs @@ -12,3 +12,4 @@ pub mod verifier; // TODO(loader_v2): Remove when we no longer need the dummy implementation. pub mod dummy; pub mod implementations; +pub mod publishing; diff --git a/third_party/move/move-vm/runtime/src/storage/module_storage.rs b/third_party/move/move-vm/runtime/src/storage/module_storage.rs index 767ad4cb5b6b53..0315a1f83c46f0 100644 --- a/third_party/move/move-vm/runtime/src/storage/module_storage.rs +++ b/third_party/move/move-vm/runtime/src/storage/module_storage.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::loader::Module; +use bytes::Bytes; use move_binary_format::{errors::PartialVMResult, CompiledModule}; use move_core_types::{account_address::AccountAddress, identifier::IdentStr, metadata::Metadata}; use std::sync::Arc; @@ -17,6 +18,14 @@ pub trait ModuleStorage { module_name: &IdentStr, ) -> PartialVMResult; + /// Returns module bytes. An error is returned if there is a storage error. If + /// the module does not exist, returns [None]. + fn fetch_module_bytes( + &self, + address: &AccountAddress, + module_name: &IdentStr, + ) -> PartialVMResult>; + /// Returns the size of a module in bytes. An error is returned if the module does /// not exist, or there is a storage error. fn fetch_module_size_in_bytes( @@ -53,3 +62,13 @@ pub trait ModuleStorage { module_name: &IdentStr, ) -> PartialVMResult>; } + +/// Storage that contains serialized modules. Clients can implement this trait +/// for their own backends, so that [ModuleStorage] can be built on top of it. +pub trait ModuleBytesStorage { + fn fetch_module_bytes( + &self, + address: &AccountAddress, + module_name: &IdentStr, + ) -> PartialVMResult>; +} diff --git a/third_party/move/move-vm/runtime/src/storage/publishing.rs b/third_party/move/move-vm/runtime/src/storage/publishing.rs new file mode 100644 index 00000000000000..99044cd0b9f923 --- /dev/null +++ b/third_party/move/move-vm/runtime/src/storage/publishing.rs @@ -0,0 +1,226 @@ +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + +use crate::{ + IntoUnsyncModuleStorage, Module, ModuleBytesStorage, ModuleStorage, RuntimeEnvironment, + UnsyncModuleStorage, +}; +use bytes::Bytes; +use move_binary_format::{ + access::ModuleAccess, + compatibility::Compatibility, + errors::{verification_error, PartialVMError, PartialVMResult}, + normalized, CompiledModule, IndexKind, +}; +use move_core_types::{ + account_address::AccountAddress, + identifier::{IdentStr, Identifier}, + metadata::Metadata, + vm_status::StatusCode, +}; +use std::{ + collections::{btree_map, BTreeMap}, + sync::Arc, +}; + +struct TemporaryModuleBytesStorage<'m, M> { + temporary_storage: BTreeMap>, + module_storage: &'m M, +} + +impl<'m, M: ModuleStorage> TemporaryModuleBytesStorage<'m, M> { + fn new( + sender: &AccountAddress, + env: &RuntimeEnvironment, + compatibility: Compatibility, + module_storage: &'m M, + module_bundle: Vec, + ) -> PartialVMResult { + use btree_map::Entry::*; + + let mut temporary_storage = BTreeMap::new(); + for module_bytes in module_bundle { + let deserializer_config = &env.vm_config().deserializer_config; + let compiled_module = + CompiledModule::deserialize_with_config(&module_bytes, deserializer_config)?; + let addr = compiled_module.self_addr(); + let name = compiled_module.self_name(); + + if addr != sender { + return Err(verification_error( + StatusCode::MODULE_ADDRESS_DOES_NOT_MATCH_SENDER, + IndexKind::AddressIdentifier, + compiled_module.self_handle_idx().0, + )); + } + + let module_exists = module_storage.check_module_exists(addr, name)?; + if module_exists && compatibility.need_check_compat() { + let old_module_ref = module_storage.fetch_verified_module(addr, name)?; + let old_module = old_module_ref.module(); + if env.vm_config().use_compatibility_checker_v2 { + compatibility.check(old_module, &compiled_module)? + } else { + #[allow(deprecated)] + let old_m = normalized::Module::new(old_module)?; + #[allow(deprecated)] + let new_m = normalized::Module::new(&compiled_module)?; + compatibility.legacy_check(&old_m, &new_m)? + } + } + + let account_module_storage = match temporary_storage.entry(*compiled_module.self_addr()) + { + Occupied(entry) => entry.into_mut(), + Vacant(entry) => entry.insert(BTreeMap::new()), + }; + + account_module_storage + .insert( + compiled_module.self_name().to_owned(), + (module_bytes, compiled_module), + ) + .ok_or_else(|| PartialVMError::new(StatusCode::DUPLICATE_MODULE_NAME))?; + } + + Ok(Self { + temporary_storage, + module_storage, + }) + } + + fn staged_modules_iter(&self) -> impl Iterator { + self.temporary_storage + .iter() + .flat_map(|(addr, account_storage)| { + account_storage + .iter() + .map(move |(name, _)| (addr, name.as_ident_str())) + }) + } +} + +impl<'m, M: ModuleStorage> ModuleBytesStorage for TemporaryModuleBytesStorage<'m, M> { + fn fetch_module_bytes( + &self, + address: &AccountAddress, + module_name: &IdentStr, + ) -> PartialVMResult> { + if let Some(account_storage) = self.temporary_storage.get(address) { + if let Some((bytes, _)) = account_storage.get(module_name) { + return Ok(Some(bytes.clone())); + } + } + self.module_storage.fetch_module_bytes(address, module_name) + } +} + +pub struct TemporaryModuleStorage<'a, M> { + storage: UnsyncModuleStorage<'a, TemporaryModuleBytesStorage<'a, M>>, +} + +impl<'a, M: ModuleStorage> TemporaryModuleStorage<'a, M> { + pub fn new( + sender: &AccountAddress, + env: &'a RuntimeEnvironment, + existing_module_storage: &'a M, + module_bundle: Vec, + ) -> PartialVMResult { + Self::new_with_compat_config( + sender, + env, + Compatibility::full_check(), + existing_module_storage, + module_bundle, + ) + } + + pub fn new_with_compat_config( + sender: &AccountAddress, + env: &'a RuntimeEnvironment, + compatibility: Compatibility, + existing_module_storage: &'a M, + module_bundle: Vec, + ) -> PartialVMResult { + let temporary_module_bytes_storage = TemporaryModuleBytesStorage::new( + sender, + env, + compatibility, + existing_module_storage, + module_bundle, + )?; + let temporary_module_storage = + temporary_module_bytes_storage.into_unsync_module_storage(env); + + // Verify the bundle, performing linking checks (e.g., no cyclic dependencies). + for (addr, name) in temporary_module_storage + .byte_storage() + .staged_modules_iter() + { + temporary_module_storage.fetch_verified_module(addr, name)?; + } + + Ok(Self { + storage: temporary_module_storage, + }) + } + + pub fn release_verified_module_bundle(self) -> impl Iterator { + self.storage + .release_byte_storage() + .temporary_storage + .into_iter() + .flat_map(|(_, account_storage)| account_storage.into_values()) + } +} + +impl<'a, M: ModuleStorage> ModuleStorage for TemporaryModuleStorage<'a, M> { + fn check_module_exists( + &self, + address: &AccountAddress, + module_name: &IdentStr, + ) -> PartialVMResult { + self.storage.check_module_exists(address, module_name) + } + + fn fetch_module_bytes( + &self, + address: &AccountAddress, + module_name: &IdentStr, + ) -> PartialVMResult> { + self.storage.fetch_module_bytes(address, module_name) + } + + fn fetch_module_size_in_bytes( + &self, + address: &AccountAddress, + module_name: &IdentStr, + ) -> PartialVMResult { + self.storage + .fetch_module_size_in_bytes(address, module_name) + } + + fn fetch_module_metadata( + &self, + address: &AccountAddress, + module_name: &IdentStr, + ) -> PartialVMResult> { + self.storage.fetch_module_metadata(address, module_name) + } + + fn fetch_deserialized_module( + &self, + address: &AccountAddress, + module_name: &IdentStr, + ) -> PartialVMResult> { + self.storage.fetch_deserialized_module(address, module_name) + } + + fn fetch_verified_module( + &self, + address: &AccountAddress, + module_name: &IdentStr, + ) -> PartialVMResult> { + self.storage.fetch_verified_module(address, module_name) + } +} diff --git a/third_party/move/testing-infra/transactional-test-runner/src/vm_test_harness.rs b/third_party/move/testing-infra/transactional-test-runner/src/vm_test_harness.rs index db98635df10ab2..e52a7c5c9a11fa 100644 --- a/third_party/move/testing-infra/transactional-test-runner/src/vm_test_harness.rs +++ b/third_party/move/testing-infra/transactional-test-runner/src/vm_test_harness.rs @@ -9,7 +9,11 @@ use crate::{ use anyhow::{anyhow, bail, Result}; use clap::Parser; use move_binary_format::{ - compatibility::Compatibility, file_format::CompiledScript, file_format_common, CompiledModule, + access::ModuleAccess, + compatibility::Compatibility, + errors::{Location, VMResult}, + file_format::CompiledScript, + file_format_common, CompiledModule, }; use move_bytecode_verifier::VerifierConfig; use move_command_line_common::{ @@ -28,7 +32,6 @@ use move_compiler::{ }; use move_core_types::{ account_address::AccountAddress, - effects::ChangeSet, identifier::{IdentStr, Identifier}, language_storage::{ModuleId, StructTag, TypeTag}, value::MoveValue, @@ -38,8 +41,12 @@ use move_resource_viewer::MoveValueAnnotator; use move_stdlib::move_stdlib_named_addresses; use move_symbol_pool::Symbol; use move_vm_runtime::{ - config::VMConfig, module_traversal::*, move_vm::MoveVM, session::SerializedReturnValues, - DummyCodeStorage, IntoUnsyncCodeStorage, IntoUnsyncModuleStorage, LocalModuleBytesStorage, + config::VMConfig, + module_traversal::*, + move_vm::MoveVM, + session::{SerializedReturnValues, Session}, + IntoUnsyncCodeStorage, IntoUnsyncModuleStorage, LocalModuleBytesStorage, + TemporaryModuleStorage, }; use move_vm_test_utils::{ gas_schedule::{CostTable, Gas, GasStatus}, @@ -51,6 +58,7 @@ use std::{ collections::{BTreeMap, BTreeSet}, iter::Iterator, path::Path, + rc::Rc, }; const STD_ADDR: AccountAddress = AccountAddress::ONE; @@ -59,7 +67,7 @@ struct SimpleVMTestAdapter<'a> { compiled_state: CompiledState<'a>, // VM to be shared by all tasks. If we use V1 loader, we store None here. - vm: Option, + vm: Option>, // Different storages for a task: resources, modules, and scripts. Module // and script storages are only used if loader V2 implementation is enabled. @@ -147,40 +155,10 @@ impl<'a> MoveTestAdapter<'a> for SimpleVMTestAdapter<'a> { named_address_mapping.insert(name, addr); } - // Create the VM and storages to initialize. - let vm = Self::create_vm(); - let mut resource_storage = InMemoryStorage::new(); - let mut module_bytes_storage = LocalModuleBytesStorage::empty(); - - // Initialize module storages. - for module in either_or_no_modules(pre_compiled_deps_v1, pre_compiled_deps_v2) - .into_iter() - .map(|tmod| &tmod.named_module.module) - { - let mut session = vm.new_session(&resource_storage); - session - .verify_module_bundle_before_publishing( - &[module.clone()], - module.self_addr(), - &DummyCodeStorage, - ) - .unwrap(); - drop(session); - - let mut module_bytes = vec![]; - module - .serialize_for_version(Some(file_format_common::VERSION_MAX), &mut module_bytes) - .unwrap(); - - // We need to store both in resource and module storage for preserving - // V1 and V2 flows. - resource_storage.publish_or_overwrite_module(module.self_id(), module_bytes.clone()); - module_bytes_storage.add_module_bytes( - module.self_addr(), - module.self_name(), - module_bytes.into(), - ); - } + let vm_config = vm_config(); + let vm = vm_config + .use_loader_v2 + .then_some(Rc::new(create_vm(vm_config))); let mut adapter = Self { compiled_state: CompiledState::new( @@ -190,14 +168,61 @@ impl<'a> MoveTestAdapter<'a> for SimpleVMTestAdapter<'a> { None, ), default_syntax, - // If we use V2 loader, we should share the VM with other tasks. - vm: vm.vm_config().use_loader_v2.then_some(vm), - resource_storage, - module_bytes_storage, comparison_mode, run_config, + vm, + module_bytes_storage: LocalModuleBytesStorage::empty(), + resource_storage: InMemoryStorage::new(), }; + let vm = adapter.vm(); + let mut modules_to_publish = vec![]; + + for module in either_or_no_modules(pre_compiled_deps_v1, pre_compiled_deps_v2) + .into_iter() + .map(|tmod| &tmod.named_module.module) + { + let mut module_bytes = vec![]; + module + .serialize_for_version(Some(file_format_common::VERSION_MAX), &mut module_bytes) + .unwrap(); + let id = module.self_id(); + let sender = *id.address(); + + let module_storage = adapter + .module_bytes_storage + .clone() + .into_unsync_module_storage(vm.runtime_env()); + + let additional_modules_to_publish = adapter + .perform_session_action(None, |session, gas_status| { + if vm.vm_config().use_loader_v2 { + let tmp_storage = TemporaryModuleStorage::new( + &sender, + vm.runtime_env(), + &module_storage, + vec![module_bytes.into()], + ) + .expect("All modules for initialization should publish"); + Ok(tmp_storage.release_verified_module_bundle().collect()) + } else { + #[allow(deprecated)] + session + .publish_module(module_bytes, sender, gas_status) + .unwrap(); + Ok(vec![]) + } + }) + .unwrap(); + modules_to_publish.extend(additional_modules_to_publish); + } + + for (bytes, module) in modules_to_publish { + adapter + .module_bytes_storage + .add_module_bytes(module.address(), module.name(), bytes); + } + let mut addr_to_name_mapping = BTreeMap::new(); for (name, addr) in move_stdlib_named_addresses() { let prev = addr_to_name_mapping.insert(addr, Symbol::from(name)); @@ -221,62 +246,69 @@ impl<'a> MoveTestAdapter<'a> for SimpleVMTestAdapter<'a> { &mut self, module: CompiledModule, _named_addr_opt: Option, - _gas_budget: Option, + gas_budget: Option, extra_args: Self::ExtraPublishArgs, ) -> Result<(Option, CompiledModule)> { - let compat = if extra_args.skip_check_struct_and_pub_function_linking { - Compatibility::no_check() - } else { - Compatibility::new( - !extra_args.skip_check_struct_layout, - !extra_args.skip_check_friend_linking, - ) - }; + let vm = self.vm(); + let module_storage = self + .module_bytes_storage + .clone() + .into_unsync_module_storage(vm.runtime_env()); - // Run verification as a separate session. - let result = { - if let Some(vm) = &self.vm { - let mut session = vm.new_session(&self.resource_storage); - session.verify_module_bundle_before_publishing_with_compat_config( - &[module.clone()], - module.self_addr(), - &DummyCodeStorage, + let mut module_bytes = vec![]; + module.serialize_for_version(Some(file_format_common::VERSION_MAX), &mut module_bytes)?; + + let id = module.self_id(); + let sender = *id.address(); + let verbose = extra_args.verbose; + let result = self.perform_session_action(gas_budget, |session, gas_status| { + let compat = if extra_args.skip_check_struct_and_pub_function_linking { + Compatibility::no_check() + } else { + Compatibility::new( + !extra_args.skip_check_struct_layout, + !extra_args.skip_check_friend_linking, + ) + }; + if vm.vm_config().use_loader_v2 { + let tmp_module_storage = TemporaryModuleStorage::new_with_compat_config( + &sender, + vm.runtime_env(), compat, + &module_storage, + vec![module_bytes.into()], ) + .map_err(|e| e.finish(Location::Undefined))?; + Ok(tmp_module_storage + .release_verified_module_bundle() + .collect()) } else { - let vm = Self::create_vm(); - let mut session = vm.new_session(&self.resource_storage); - session.verify_module_bundle_before_publishing_with_compat_config( - &[module.clone()], - module.self_addr(), - &DummyCodeStorage, + #[allow(deprecated)] + session.publish_module_bundle_with_compat_config( + vec![module_bytes], + sender, + gas_status, compat, - ) + )?; + Ok(vec![]) } - }; - + }); match result { - Ok(()) => { - let mut module_bytes = vec![]; - module.serialize_for_version( - Some(file_format_common::VERSION_MAX), - &mut module_bytes, - )?; - - self.module_bytes_storage.add_module_bytes( - module.self_addr(), - module.self_name(), - module_bytes.clone().into(), - ); - self.resource_storage - .publish_or_overwrite_module(module.self_id(), module_bytes); + Ok(modules_to_publish) => { + for (bytes, module) in modules_to_publish { + self.module_bytes_storage.add_module_bytes( + module.address(), + module.name(), + bytes, + ); + } Ok((None, module)) }, Err(vm_error) => Err(anyhow!( "Unable to publish module '{}'. Got VMError: {}", module.self_id(), vm_error.format_test_output( - move_test_debug() || extra_args.verbose, + move_test_debug() || verbose, !move_test_debug() && self.comparison_mode ) )), @@ -292,6 +324,12 @@ impl<'a> MoveTestAdapter<'a> for SimpleVMTestAdapter<'a> { gas_budget: Option, extra_args: Self::ExtraRunArgs, ) -> Result> { + let vm = self.vm(); + let module_and_script_storage = self + .module_bytes_storage + .clone() + .into_unsync_code_storage(vm.runtime_env()); + let signers: Vec<_> = signers .into_iter() .map(|addr| self.compiled_state().resolve_address(&addr)) @@ -310,28 +348,28 @@ impl<'a> MoveTestAdapter<'a> for SimpleVMTestAdapter<'a> { .map(|a| MoveValue::Signer(*a).simple_serialize().unwrap()) .chain(args) .collect(); - - let change_set = if let Some(vm) = &self.vm { - self.execute_script_impl( - vm, - &script_bytes, - type_args, - args, - gas_budget, - extra_args.verbose, - )? - } else { - let vm = Self::create_vm(); - self.execute_script_impl( - &vm, - &script_bytes, + let verbose = extra_args.verbose; + let traversal_storage = TraversalStorage::new(); + self.perform_session_action(gas_budget, |session, gas_status| { + session.execute_script( + script_bytes, type_args, args, - gas_budget, - extra_args.verbose, - )? - }; - self.resource_storage.apply(change_set).unwrap(); + gas_status, + &mut TraversalContext::new(&traversal_storage), + &module_and_script_storage, + &module_and_script_storage, + ) + }) + .map_err(|vm_error| { + anyhow!( + "Script execution failed with VMError: {}", + vm_error.format_test_output( + move_test_debug() || verbose, + !move_test_debug() && self.comparison_mode + ) + ) + })?; Ok(None) } @@ -345,6 +383,12 @@ impl<'a> MoveTestAdapter<'a> for SimpleVMTestAdapter<'a> { gas_budget: Option, extra_args: Self::ExtraRunArgs, ) -> Result<(Option, SerializedReturnValues)> { + let vm = self.vm(); + let module_storage = self + .module_bytes_storage + .clone() + .into_unsync_module_storage(vm.runtime_env()); + let signers: Vec<_> = signers .into_iter() .map(|addr| self.compiled_state().resolve_address(&addr)) @@ -360,30 +404,29 @@ impl<'a> MoveTestAdapter<'a> for SimpleVMTestAdapter<'a> { .map(|a| MoveValue::Signer(*a).simple_serialize().unwrap()) .chain(args) .collect(); - - let (serialized_return_values, change_set) = if let Some(vm) = &self.vm { - self.call_function_impl( - vm, - module, - function, - type_args, - args, - gas_budget, - extra_args.verbose, - )? - } else { - let vm = Self::create_vm(); - self.call_function_impl( - &vm, - module, - function, - type_args, - args, - gas_budget, - extra_args.verbose, - )? - }; - self.resource_storage.apply(change_set).unwrap(); + let verbose = extra_args.verbose; + let traversal_storage = TraversalStorage::new(); + let serialized_return_values = self + .perform_session_action(gas_budget, |session, gas_status| { + session.execute_function_bypass_visibility( + module, + function, + type_args, + args, + gas_status, + &mut TraversalContext::new(&traversal_storage), + &module_storage, + ) + }) + .map_err(|vm_error| { + anyhow!( + "Function execution failed with VMError: {}", + vm_error.format_test_output( + move_test_debug() || verbose, + !move_test_debug() && self.comparison_mode + ) + ) + })?; Ok((None, serialized_return_values)) } @@ -421,112 +464,58 @@ impl<'a> MoveTestAdapter<'a> for SimpleVMTestAdapter<'a> { } impl<'a> SimpleVMTestAdapter<'a> { - fn create_vm() -> MoveVM { - let vm_config = VMConfig { - verifier_config: VerifierConfig::production(), - paranoid_type_checks: true, - ..VMConfig::default() - }; - MoveVM::new_with_config( - move_stdlib::natives::all_natives( - STD_ADDR, - // TODO: come up with a suitable gas schedule - move_stdlib::natives::GasParameters::zeros(), - ), - vm_config, - ) - } - - fn get_gas_status(&self, gas_budget: Option) -> GasStatus { - get_gas_status( - &move_vm_test_utils::gas_schedule::INITIAL_COST_SCHEDULE, - gas_budget, - ) - .unwrap() + fn vm(&self) -> Rc { + match &self.vm { + Some(vm) => vm.clone(), + None => Rc::new(create_vm(vm_config())), + } } - fn execute_script_impl( - &self, - vm: &MoveVM, - script_bytes: &[u8], - ty_args: Vec, - args: Vec>, + fn perform_session_action( + &mut self, gas_budget: Option, - verbose: bool, - ) -> anyhow::Result { - let traversal_storage = TraversalStorage::new(); - let mut gas_status = self.get_gas_status(gas_budget); - - let mut session = vm.new_session(&self.resource_storage); - let module_and_script_storage = self - .module_bytes_storage - .clone() - .into_unsync_code_storage(vm.runtime_env()); - session - .execute_script( - script_bytes, - ty_args, - args, - &mut gas_status, - &mut TraversalContext::new(&traversal_storage), - &module_and_script_storage, - &module_and_script_storage, + f: impl FnOnce(&mut Session, &mut GasStatus) -> VMResult, + ) -> VMResult { + let vm = self.vm(); + let (mut session, mut gas_status) = { + let gas_status = get_gas_status( + &move_vm_test_utils::gas_schedule::INITIAL_COST_SCHEDULE, + gas_budget, ) - .map_err(|vm_error| { - anyhow!( - "Script execution failed with VMError: {}", - vm_error.format_test_output( - move_test_debug() || verbose, - !move_test_debug() && self.comparison_mode - ) - ) - })?; - let change_set = session.finish()?; - Ok(change_set) - } + .unwrap(); + let session = vm.new_session(&self.resource_storage); + (session, gas_status) + }; - fn call_function_impl( - &self, - vm: &MoveVM, - module_id: &ModuleId, - function_name: &IdentStr, - ty_args: Vec, - args: Vec>, - gas_budget: Option, - verbose: bool, - ) -> anyhow::Result<(SerializedReturnValues, ChangeSet)> { - let traversal_storage = TraversalStorage::new(); - let mut gas_status = self.get_gas_status(gas_budget); + // perform op + let res = f(&mut session, &mut gas_status)?; - let mut session = vm.new_session(&self.resource_storage); - let module_storage = self - .module_bytes_storage - .clone() - .into_unsync_module_storage(vm.runtime_env()); - let results = session - .execute_function_bypass_visibility( - module_id, - function_name, - ty_args, - args, - &mut gas_status, - &mut TraversalContext::new(&traversal_storage), - &module_storage, - ) - .map_err(|vm_error| { - anyhow!( - "Function execution failed with VMError: {}", - vm_error.format_test_output( - move_test_debug() || verbose, - !move_test_debug() && self.comparison_mode - ) - ) - })?; - let change_set = session.finish()?; - Ok((results, change_set)) + // save changeset + let changeset = session.finish()?; + self.resource_storage.apply(changeset).unwrap(); + Ok(res) } } +fn vm_config() -> VMConfig { + VMConfig { + verifier_config: VerifierConfig::production(), + paranoid_type_checks: true, + ..VMConfig::default() + } +} + +fn create_vm(vm_config: VMConfig) -> MoveVM { + MoveVM::new_with_config( + move_stdlib::natives::all_natives( + STD_ADDR, + // TODO: come up with a suitable gas schedule + move_stdlib::natives::GasParameters::zeros(), + ), + vm_config, + ) +} + fn get_gas_status(cost_table: &CostTable, gas_budget: Option) -> Result { let gas_status = if let Some(gas_budget) = gas_budget { // TODO(Gas): This should not be hardcoded. diff --git a/types/src/transaction/module.rs b/types/src/transaction/module.rs index 49b777de996ef5..4dd28ef95ad52f 100644 --- a/types/src/transaction/module.rs +++ b/types/src/transaction/module.rs @@ -2,6 +2,7 @@ // Parts of the project are originally copyright © Meta Platforms, Inc. // SPDX-License-Identifier: Apache-2.0 +use bytes::Bytes; use serde::{Deserialize, Serialize}; use std::fmt; @@ -55,6 +56,13 @@ impl ModuleBundle { self.codes.into_iter().map(Module::into_inner).collect() } + pub fn into_bytes(self) -> Vec { + self.codes + .into_iter() + .map(|m| m.into_inner().into()) + .collect() + } + pub fn iter(&self) -> impl Iterator { self.codes.iter() }