diff --git a/Cargo.lock b/Cargo.lock index 58fba5ade075b..102d4687d0480 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3134,6 +3134,10 @@ dependencies = [ "tiny-keccak", ] +[[package]] +name = "aptos-utils" +version = "0.1.0" + [[package]] name = "aptos-validator-interface" version = "0.1.0" @@ -3189,6 +3193,7 @@ dependencies = [ "aptos-mvhashmap", "aptos-state-view", "aptos-types", + "aptos-utils", "aptos-vm-logging", "bcs 0.1.4 (git+https://github.com/aptos-labs/bcs.git?rev=d31fab9d81748e2594be5cd5cdf845786a30562d)", "dashmap", diff --git a/Cargo.toml b/Cargo.toml index 12d45d98e3573..12e3cfd67aa84 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ members = [ "aptos-move/vm-genesis", "aptos-move/writeset-transaction-generator", "aptos-node", + "aptos-utils", "config", "config/global-constants", "consensus", @@ -358,6 +359,7 @@ aptos-transaction-emitter-lib = { path = "crates/transaction-emitter-lib" } aptos-transaction-generator-lib = { path = "crates/transaction-generator-lib" } aptos-transactional-test-harness = { path = "aptos-move/aptos-transactional-test-harness" } aptos-types = { path = "types" } +aptos-utils = { path = "aptos-utils" } aptos-validator-interface = { path = "aptos-move/aptos-validator-interface" } aptos-vault-client = { path = "secure/storage/vault" } aptos-vm = { path = "aptos-move/aptos-vm" } diff --git a/aptos-move/aptos-debugger/src/lib.rs b/aptos-move/aptos-debugger/src/lib.rs index 2dc40c7c7d1be..2a4d2ea081a42 100644 --- a/aptos-move/aptos-debugger/src/lib.rs +++ b/aptos-move/aptos-debugger/src/lib.rs @@ -220,7 +220,7 @@ impl AptosDebugger { pub fn run_session_at_version(&self, version: Version, f: F) -> Result where - F: FnOnce(&mut SessionExt>) -> VMResult<()>, + F: FnOnce(&mut SessionExt) -> VMResult<()>, { let state_view = DebuggerStateView::new(self.debugger.clone(), version); let state_view_storage = StorageAdapter::new(&state_view); diff --git a/aptos-move/aptos-vm/Cargo.toml b/aptos-move/aptos-vm/Cargo.toml index 4402d266d686a..f920c5eca763e 100644 --- a/aptos-move/aptos-vm/Cargo.toml +++ b/aptos-move/aptos-vm/Cargo.toml @@ -27,6 +27,7 @@ aptos-move-stdlib = { workspace = true } aptos-mvhashmap = { workspace = true } aptos-state-view = { workspace = true } aptos-types = { workspace = true } +aptos-utils = { workspace = true } aptos-vm-logging = { workspace = true } bcs = { workspace = true } dashmap = { workspace = true } diff --git a/aptos-move/aptos-vm/src/adapter_common.rs b/aptos-move/aptos-vm/src/adapter_common.rs index 6c67f12f9ac9b..515fbdf3cc97a 100644 --- a/aptos-move/aptos-vm/src/adapter_common.rs +++ b/aptos-move/aptos-vm/src/adapter_common.rs @@ -18,15 +18,15 @@ use aptos_vm_logging::log_schema::AdapterLogSchema; /// This trait describes the VM adapter's interface. /// TODO: bring more of the execution logic in aptos_vm into this file. -pub trait VMAdapter { +pub(crate) trait VMAdapter { /// Creates a new Session backed by the given storage. /// TODO: this doesn't belong in this trait. We should be able to remove /// this after redesigning cache ownership model. - fn new_session<'r, R: MoveResolverExt>( + fn new_session<'r>( &self, - remote: &'r R, + remote: &'r impl MoveResolverExt, session_id: SessionId, - ) -> SessionExt<'r, '_, R>; + ) -> SessionExt<'r, '_>; /// Checks the signature of the given signed transaction and returns /// `Ok(SignatureCheckedTransaction)` if the signature is valid. @@ -36,10 +36,10 @@ pub trait VMAdapter { fn check_transaction_format(&self, txn: &SignedTransaction) -> Result<(), VMStatus>; /// Runs the prologue for the given transaction. - fn run_prologue( + fn run_prologue( &self, - session: &mut SessionExt, - storage: &S, + session: &mut SessionExt, + storage: &impl MoveResolverExt, transaction: &SignatureCheckedTransaction, log_context: &AdapterLogSchema, ) -> Result<(), VMStatus>; @@ -48,17 +48,17 @@ pub trait VMAdapter { fn should_restart_execution(output: &TransactionOutput) -> bool; /// Execute a single transaction. - fn execute_single_transaction( + fn execute_single_transaction( &self, txn: &PreprocessedTransaction, - data_cache: &S, + data_cache: &impl MoveResolverExt, log_context: &AdapterLogSchema, ) -> Result<(VMStatus, TransactionOutputExt, Option), VMStatus>; - fn validate_signature_checked_transaction( + fn validate_signature_checked_transaction( &self, - session: &mut SessionExt, - storage: &S, + session: &mut SessionExt, + storage: &impl MoveResolverExt, transaction: &SignatureCheckedTransaction, allow_too_new: bool, log_context: &AdapterLogSchema, diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs index d61956fdfbd14..70c6c7f1858ab 100644 --- a/aptos-move/aptos-vm/src/aptos_vm.rs +++ b/aptos-move/aptos-vm/src/aptos_vm.rs @@ -44,6 +44,7 @@ use aptos_types::{ vm_status::{AbortLocation, DiscardedVMStatus, StatusCode, VMStatus}, write_set::WriteSet, }; +use aptos_utils::{aptos_try, return_on_failure}; use aptos_vm_logging::{ init_speculative_logs, log_schema::AdapterLogSchema, speculative_error, speculative_log, }; @@ -90,7 +91,6 @@ pub fn allow_module_bundle_for_test() { MODULE_BUNDLE_DISALLOWED.store(false, Ordering::Relaxed); } -#[derive(Clone)] pub struct AptosVM(pub(crate) AptosVMImpl); struct AptosSimulationVM(AptosVM); @@ -105,11 +105,11 @@ macro_rules! unwrap_or_discard { } impl AptosVM { - pub fn new(state: &S) -> Self { + pub fn new(state: &impl StateView) -> Self { Self(AptosVMImpl::new(state)) } - pub fn new_for_validation(state: &S) -> Self { + pub fn new_for_validation(state: &impl StateView) -> Self { info!( AdapterLogSchema::new(state.id(), 0), "Adapter created for Validation" @@ -194,22 +194,22 @@ impl AptosVM { } /// Load a module into its internal MoveVM's code cache. - pub fn load_module( + pub fn load_module( &self, module_id: &ModuleId, - state: &S, + state: &impl MoveResolverExt, ) -> VMResult> { self.0.load_module(module_id, state) } /// Generates a transaction output for a transaction that encountered errors during the /// execution process. This is public for now only for tests. - pub fn failed_transaction_cleanup( + pub fn failed_transaction_cleanup( &self, error_code: VMStatus, gas_meter: &mut impl AptosGasMeter, txn_data: &TransactionMetadata, - storage: &S, + storage: &impl MoveResolverExt, log_context: &AdapterLogSchema, change_set_configs: &ChangeSetConfigs, ) -> TransactionOutputExt { @@ -224,12 +224,12 @@ impl AptosVM { .1 } - fn failed_transaction_cleanup_and_keep_vm_status( + fn failed_transaction_cleanup_and_keep_vm_status( &self, error_code: VMStatus, gas_meter: &mut impl AptosGasMeter, txn_data: &TransactionMetadata, - storage: &S, + storage: &impl MoveResolverExt, log_context: &AdapterLogSchema, change_set_configs: &ChangeSetConfigs, ) -> (VMStatus, TransactionOutputExt) { @@ -286,9 +286,9 @@ impl AptosVM { } } - fn success_transaction_cleanup( + fn success_transaction_cleanup( &self, - storage: &S, + storage: &impl MoveResolverExt, user_txn_change_set_ext: ChangeSetExt, gas_meter: &mut impl AptosGasMeter, txn_data: &TransactionMetadata, @@ -318,9 +318,7 @@ impl AptosVM { self.0 .run_success_epilogue(&mut session, gas_meter.balance(), txn_data, log_context)?; - let epilogue_change_set_ext = session - .finish(&mut (), change_set_configs) - .map_err(|e| e.into_vm_status())?; + let epilogue_change_set_ext = session.finish(&mut (), change_set_configs)?; let change_set_ext = user_txn_change_set_ext .squash(epilogue_change_set_ext) .map_err(|_err| VMStatus::Error(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, None))?; @@ -346,9 +344,9 @@ impl AptosVM { )) } - fn validate_and_execute_entry_function( + fn validate_and_execute_entry_function( &self, - session: &mut SessionExt, + session: &mut SessionExt, gas_meter: &mut impl AptosGasMeter, senders: Vec, script_fn: &EntryFunction, @@ -369,21 +367,19 @@ impl AptosVM { &function, struct_constructors, )?; - session - .execute_entry_function( - script_fn.module(), - script_fn.function(), - script_fn.ty_args().to_vec(), - args, - gas_meter, - ) - .map_err(|e| e.into_vm_status()) + Ok(session.execute_entry_function( + script_fn.module(), + script_fn.function(), + script_fn.ty_args().to_vec(), + args, + gas_meter, + )?) } - fn execute_script_or_entry_function( + fn execute_script_or_entry_function( &self, - storage: &S, - mut session: SessionExt, + storage: &impl MoveResolverExt, + mut session: SessionExt, gas_meter: &mut impl AptosGasMeter, txn_data: &TransactionMetadata, payload: &TransactionPayload, @@ -400,9 +396,7 @@ impl AptosVM { // Run the execution logic { - gas_meter - .charge_intrinsic_gas_for_transaction(txn_data.transaction_size()) - .map_err(|e| e.into_vm_status())?; + gas_meter.charge_intrinsic_gas_for_transaction(txn_data.transaction_size())?; match payload { TransactionPayload::Script(script) => { @@ -420,9 +414,12 @@ impl AptosVM { .get_features() .is_enabled(FeatureFlag::STRUCT_CONSTRUCTORS), )?; - session - .execute_script(script.code(), script.ty_args().to_vec(), args, gas_meter) - .map_err(|e| e.into_vm_status())?; + session.execute_script( + script.code(), + script.ty_args().to_vec(), + args, + gas_meter, + )?; }, TransactionPayload::EntryFunction(script_fn) => { let mut senders = vec![txn_data.sender()]; @@ -449,9 +446,7 @@ impl AptosVM { new_published_modules_loaded, )?; - let change_set_ext = session - .finish(&mut (), change_set_configs) - .map_err(|e| e.into_vm_status())?; + let change_set_ext = session.finish(&mut (), change_set_configs)?; gas_meter.charge_io_gas_for_write_set(change_set_ext.write_set().iter())?; gas_meter.charge_storage_fee_for_all( change_set_ext.write_set().iter(), @@ -479,10 +474,10 @@ impl AptosVM { // failure object. In case of success, keep the session and also do any necessary module publish // cleanup. // 3. Call post transaction cleanup function in multisig account module with the result from (2) - fn execute_multisig_transaction( + fn execute_multisig_transaction( &self, - storage: &S, - mut session: SessionExt, + storage: &impl MoveResolverExt, + mut session: SessionExt, gas_meter: &mut impl AptosGasMeter, txn_data: &TransactionMetadata, txn_payload: &Multisig, @@ -497,9 +492,7 @@ impl AptosVM { )) }); - gas_meter - .charge_intrinsic_gas_for_transaction(txn_data.transaction_size()) - .map_err(|e| e.into_vm_status())?; + gas_meter.charge_intrinsic_gas_for_transaction(txn_data.transaction_size())?; // Step 1: Obtain the payload. If any errors happen here, the entire transaction should fail let invariant_violation_error = @@ -609,9 +602,9 @@ impl AptosVM { ) } - fn execute_multisig_entry_function( + fn execute_multisig_entry_function( &self, - session: &mut SessionExt, + session: &mut SessionExt, gas_meter: &mut impl AptosGasMeter, multisig_address: AccountAddress, payload: &EntryFunction, @@ -632,10 +625,10 @@ impl AptosVM { Ok(()) } - fn success_multisig_payload_cleanup( + fn success_multisig_payload_cleanup( &self, - storage: &S, - session: SessionExt, + storage: &impl MoveResolverExt, + session: SessionExt, gas_meter: &mut impl AptosGasMeter, txn_data: &TransactionMetadata, cleanup_args: Vec>, @@ -644,9 +637,7 @@ impl AptosVM { // Charge gas for writeset before we do cleanup. This ensures we don't charge gas for // cleanup writeset changes, which is consistent with outer-level success cleanup // flow. We also wouldn't need to worry that we run out of gas when doing cleanup. - let inner_function_change_set_ext = session - .finish(&mut (), change_set_configs) - .map_err(|e| e.into_vm_status())?; + let inner_function_change_set_ext = session.finish(&mut (), change_set_configs)?; gas_meter.charge_io_gas_for_write_set(inner_function_change_set_ext.write_set().iter())?; gas_meter.charge_storage_fee_for_all( inner_function_change_set_ext.write_set().iter(), @@ -672,18 +663,16 @@ impl AptosVM { cleanup_args, &mut UnmeteredGasMeter, )?; - let cleanup_change_set_ext = cleanup_session - .finish(&mut (), change_set_configs) - .map_err(|e| e.into_vm_status())?; + let cleanup_change_set_ext = cleanup_session.finish(&mut (), change_set_configs)?; // Merge the inner function writeset with cleanup writeset. inner_function_change_set_ext .squash(cleanup_change_set_ext) .map_err(|_err| VMStatus::Error(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR, None)) } - fn failure_multisig_payload_cleanup( + fn failure_multisig_payload_cleanup( &self, - storage: &S, + storage: &impl MoveResolverExt, execution_error: VMStatus, txn_data: &TransactionMetadata, mut cleanup_args: Vec>, @@ -706,13 +695,11 @@ impl AptosVM { cleanup_args, &mut UnmeteredGasMeter, )?; - cleanup_session - .finish(&mut (), change_set_configs) - .map_err(|e| e.into_vm_status()) + Ok(cleanup_session.finish(&mut (), change_set_configs)?) } - fn verify_module_bundle( - session: &mut SessionExt, + fn verify_module_bundle( + session: &mut SessionExt, module_bundle: &ModuleBundle, ) -> VMResult<()> { for module_blob in module_bundle.iter() { @@ -735,9 +722,9 @@ impl AptosVM { } /// Execute all module initializers. - fn execute_module_initialization( + fn execute_module_initialization( &self, - session: &mut SessionExt, + session: &mut SessionExt, gas_meter: &mut impl AptosGasMeter, modules: &[CompiledModule], exists: BTreeSet, @@ -808,10 +795,10 @@ impl AptosVM { /// Execute a module bundle load request. /// TODO: this is going to be deprecated and removed in favor of code publishing via /// NativeCodeContext - fn execute_modules( + fn execute_modules( &self, - storage: &S, - mut session: SessionExt, + storage: &impl MoveResolverExt, + mut session: SessionExt, gas_meter: &mut impl AptosGasMeter, txn_data: &TransactionMetadata, modules: &ModuleBundle, @@ -829,26 +816,22 @@ impl AptosVM { )) }); - gas_meter - .charge_intrinsic_gas_for_transaction(txn_data.transaction_size()) - .map_err(|e| e.into_vm_status())?; + gas_meter.charge_intrinsic_gas_for_transaction(txn_data.transaction_size())?; Self::verify_module_bundle(&mut session, modules)?; - session - .publish_module_bundle_with_compat_config( - modules.clone().into_inner(), - txn_data.sender(), - gas_meter, - Compatibility::new( - true, - true, - !self - .0 - .get_features() - .is_enabled(FeatureFlag::TREAT_FRIEND_AS_PRIVATE), - ), - ) - .map_err(|e| e.into_vm_status())?; + session.publish_module_bundle_with_compat_config( + modules.clone().into_inner(), + txn_data.sender(), + gas_meter, + Compatibility::new( + true, + true, + !self + .0 + .get_features() + .is_enabled(FeatureFlag::TREAT_FRIEND_AS_PRIVATE), + ), + )?; // call init function of the each module self.execute_module_initialization( @@ -860,9 +843,7 @@ impl AptosVM { new_published_modules_loaded, )?; - let change_set_ext = session - .finish(&mut (), change_set_configs) - .map_err(|e| e.into_vm_status())?; + let change_set_ext = session.finish(&mut (), change_set_configs)?; gas_meter.charge_io_gas_for_write_set(change_set_ext.write_set().iter())?; gas_meter.charge_storage_fee_for_all( change_set_ext.write_set().iter(), @@ -883,9 +864,9 @@ impl AptosVM { } /// Resolve a pending code publish request registered via the NativeCodeContext. - fn resolve_pending_code_publish( + fn resolve_pending_code_publish( &self, - session: &mut SessionExt, + session: &mut SessionExt, gas_meter: &mut impl AptosGasMeter, new_published_modules_loaded: &mut bool, ) -> VMResult<()> { @@ -918,39 +899,37 @@ impl AptosVM { // Publish the bundle and execute initializers // publish_module_bundle doesn't actually load the published module into // the loader cache. It only puts the module data in the data cache. - session - .publish_module_bundle_with_compat_config( - bundle.into_inner(), - destination, - gas_meter, - Compatibility::new( - true, - true, - !self - .0 - .get_features() - .is_enabled(FeatureFlag::TREAT_FRIEND_AS_PRIVATE), - ), - ) - .and_then(|_| { - self.execute_module_initialization( - session, - gas_meter, - &modules, - exists, - &[destination], - new_published_modules_loaded, - ) - }) + return_on_failure!(session.publish_module_bundle_with_compat_config( + bundle.into_inner(), + destination, + gas_meter, + Compatibility::new( + true, + true, + !self + .0 + .get_features() + .is_enabled(FeatureFlag::TREAT_FRIEND_AS_PRIVATE), + ), + )); + + self.execute_module_initialization( + session, + gas_meter, + &modules, + exists, + &[destination], + new_published_modules_loaded, + ) } else { Ok(()) } } /// Validate a publish request. - fn validate_publish_request( + fn validate_publish_request( &self, - session: &mut SessionExt, + session: &mut SessionExt, modules: &[CompiledModule], mut expected_modules: BTreeSet, allowed_deps: Option>>, @@ -1010,17 +989,13 @@ impl AptosVM { )) } - fn execute_user_transaction_impl( + fn execute_user_transaction_impl( &self, - storage: &S, + storage: &impl MoveResolverExt, txn: &SignatureCheckedTransaction, log_context: &AdapterLogSchema, - gas_meter: &mut G, - ) -> (VMStatus, TransactionOutputExt) - where - G: AptosGasMeter, - S: MoveResolverExt + StateView, - { + gas_meter: &mut impl AptosGasMeter, + ) -> (VMStatus, TransactionOutputExt) { // Revalidate the transaction. let resolver = self.0.new_move_resolver(storage); let mut session = self.0.new_session(&resolver, SessionId::txn(txn)); @@ -1129,9 +1104,9 @@ impl AptosVM { } } - pub(crate) fn execute_user_transaction( + fn execute_user_transaction( &self, - storage: &S, + storage: &impl MoveResolverExt, txn: &SignatureCheckedTransaction, log_context: &AdapterLogSchema, ) -> (VMStatus, TransactionOutputExt) { @@ -1142,14 +1117,13 @@ impl AptosVM { self.execute_user_transaction_impl(storage, txn, log_context, &mut gas_meter) } - pub fn execute_user_transaction_with_custom_gas_meter( - state_view: &S, + pub fn execute_user_transaction_with_custom_gas_meter( + state_view: &impl StateView, txn: &SignatureCheckedTransaction, log_context: &AdapterLogSchema, make_gas_meter: F, ) -> Result<(VMStatus, TransactionOutput, G), VMStatus> where - S: StateView, G: AptosGasMeter, F: FnOnce(u64, AptosGasParameters, StorageGasParameters, Gas) -> Result, { @@ -1174,23 +1148,23 @@ impl AptosVM { Ok((status, output.into_transaction_output(&storage), gas_meter)) } - fn execute_writeset( + fn execute_writeset( &self, - storage: &S, + storage: &impl MoveResolverExt, writeset_payload: &WriteSetPayload, txn_sender: Option, session_id: SessionId, - ) -> Result> { + ) -> Result { let mut gas_meter = UnmeteredGasMeter; let change_set_configs = ChangeSetConfigs::unlimited_at_gas_feature_version(self.0.get_gas_feature_version()); - Ok(match writeset_payload { - WriteSetPayload::Direct(change_set) => ChangeSetExt::new( + match writeset_payload { + WriteSetPayload::Direct(change_set) => Ok(ChangeSetExt::new( DeltaChangeSet::empty(), change_set.clone(), Arc::new(change_set_configs), - ), + )), WriteSetPayload::Script { script, execute_as } => { let resolver = self.0.new_move_resolver(storage); let mut tmp_session = self.0.new_session(&resolver, session_id); @@ -1199,9 +1173,8 @@ impl AptosVM { Some(sender) => vec![sender, *execute_as], }; - let loaded_func = tmp_session - .load_script(script.code(), script.ty_args().to_vec()) - .map_err(|e| Err(e.into_vm_status()))?; + let loaded_func = + tmp_session.load_script(script.code(), script.ty_args().to_vec())?; let args = verifier::transaction_arg_validation::validate_combine_signer_and_txn_args( &mut tmp_session, @@ -1211,20 +1184,17 @@ impl AptosVM { self.0 .get_features() .is_enabled(FeatureFlag::STRUCT_CONSTRUCTORS), - ) - .map_err(Err)?; + )?; - tmp_session - .execute_script( - script.code(), - script.ty_args().to_vec(), - args, - &mut gas_meter, - ) - .and_then(|_| tmp_session.finish(&mut (), &change_set_configs)) - .map_err(|e| Err(e.into_vm_status()))? + return_on_failure!(tmp_session.execute_script( + script.code(), + script.ty_args().to_vec(), + args, + &mut gas_meter, + )); + Ok(tmp_session.finish(&mut (), &change_set_configs)?) }, - }) + } } fn read_writeset( @@ -1265,23 +1235,20 @@ impl AptosVM { } } - pub(crate) fn process_waypoint_change_set( + pub(crate) fn process_waypoint_change_set( &self, - storage: &S, + storage: &impl MoveResolverExt, writeset_payload: WriteSetPayload, log_context: &AdapterLogSchema, ) -> Result<(VMStatus, TransactionOutputExt), VMStatus> { // TODO: user specified genesis id to distinguish different genesis write sets let genesis_id = HashValue::zero(); - let change_set_ext = match self.execute_writeset( + let change_set_ext = self.execute_writeset( storage, &writeset_payload, Some(aptos_types::account_config::reserved_vm_address()), SessionId::genesis(genesis_id), - ) { - Ok(cse) => cse, - Err(e) => return e, - }; + )?; let (delta_change_set, change_set) = change_set_ext.into_inner(); Self::validate_waypoint_change_set(&change_set, log_context)?; @@ -1296,9 +1263,9 @@ impl AptosVM { )) } - pub(crate) fn process_block_prologue( + pub(crate) fn process_block_prologue( &self, - storage: &S, + storage: &impl MoveResolverExt, block_metadata: BlockMetadata, log_context: &AdapterLogSchema, ) -> Result<(VMStatus, TransactionOutputExt), VMStatus> { @@ -1406,10 +1373,10 @@ impl AptosVM { .collect::>()) } - fn run_prologue_with_payload( + fn run_prologue_with_payload( &self, - session: &mut SessionExt, - storage: &S, + session: &mut SessionExt, + storage: &impl MoveResolverExt, payload: &TransactionPayload, txn_data: &TransactionMetadata, log_context: &AdapterLogSchema, @@ -1550,11 +1517,11 @@ impl VMValidator for AptosVM { } impl VMAdapter for AptosVM { - fn new_session<'r, R: MoveResolverExt>( + fn new_session<'r>( &self, - remote: &'r R, + remote: &'r impl MoveResolverExt, session_id: SessionId, - ) -> SessionExt<'r, '_, R> { + ) -> SessionExt<'r, '_> { self.0.new_session(remote, session_id) } @@ -1573,10 +1540,10 @@ impl VMAdapter for AptosVM { Ok(()) } - fn run_prologue( + fn run_prologue( &self, - session: &mut SessionExt, - storage: &S, + session: &mut SessionExt, + storage: &impl MoveResolverExt, transaction: &SignatureCheckedTransaction, log_context: &AdapterLogSchema, ) -> Result<(), VMStatus> { @@ -1599,10 +1566,10 @@ impl VMAdapter for AptosVM { .any(|event| *event.key() == new_epoch_event_key) } - fn execute_single_transaction( + fn execute_single_transaction( &self, txn: &PreprocessedTransaction, - data_cache: &S, + data_cache: &impl MoveResolverExt, log_context: &AdapterLogSchema, ) -> Result<(VMStatus, TransactionOutputExt, Option), VMStatus> { Ok(match txn { @@ -1684,10 +1651,10 @@ impl AsMut for AptosVM { } impl AptosSimulationVM { - fn validate_simulated_transaction( + fn validate_simulated_transaction( &self, - session: &mut SessionExt, - storage: &S, + session: &mut SessionExt, + storage: &impl MoveResolverExt, transaction: &SignedTransaction, txn_data: &TransactionMetadata, log_context: &AdapterLogSchema, @@ -1706,9 +1673,9 @@ impl AptosSimulationVM { /* Executes a SignedTransaction without performing signature verification */ - fn simulate_signed_transaction( + fn simulate_signed_transaction( &self, - storage: &S, + storage: &impl MoveResolverExt, txn: &SignedTransaction, log_context: &AdapterLogSchema, ) -> (VMStatus, TransactionOutputExt) { @@ -1765,40 +1732,41 @@ impl AptosSimulationVM { if let Some(payload) = multisig.transaction_payload.clone() { match payload { MultisigTransactionPayload::EntryFunction(entry_function) => { - self.0 - .execute_multisig_entry_function( + aptos_try!({ + return_on_failure!(self.0.execute_multisig_entry_function( &mut session, &mut gas_meter, multisig.multisig_address, &entry_function, &mut new_published_modules_loaded, + )); + // TODO: Deduplicate this against execute_multisig_transaction + // A bit tricky since we need to skip success/failure cleanups, + // which is in the middle. Introducing a boolean would make the code + // messier. + let change_set_ext = session + .finish(&mut (), &storage_gas_params.change_set_configs)?; + + return_on_failure!(gas_meter.charge_io_gas_for_write_set( + change_set_ext.write_set().iter(), + )); + + return_on_failure!(gas_meter.charge_storage_fee_for_all( + change_set_ext.write_set().iter(), + change_set_ext.change_set().events(), + txn_data.transaction_size, + txn_data.gas_unit_price, + )); + + self.0.success_transaction_cleanup( + storage, + change_set_ext, + &mut gas_meter, + &txn_data, + log_context, + &storage_gas_params.change_set_configs, ) - .and_then(|_| { - // TODO: Deduplicate this against execute_multisig_transaction - // A bit tricky since we need to skip success/failure cleanups, - // which is in the middle. Introducing a boolean would make the code - // messier. - let change_set_ext = session - .finish(&mut (), &storage_gas_params.change_set_configs) - .map_err(|e| e.into_vm_status())?; - gas_meter.charge_io_gas_for_write_set( - change_set_ext.write_set().iter(), - )?; - gas_meter.charge_storage_fee_for_all( - change_set_ext.write_set().iter(), - change_set_ext.change_set().events(), - txn_data.transaction_size, - txn_data.gas_unit_price, - )?; - self.0.success_transaction_cleanup( - storage, - change_set_ext, - &mut gas_meter, - &txn_data, - log_context, - &storage_gas_params.change_set_configs, - ) - }) + }) }, } } else { diff --git a/aptos-move/aptos-vm/src/aptos_vm_impl.rs b/aptos-move/aptos-vm/src/aptos_vm_impl.rs index 1ae936675fc26..b2b8b67f93811 100644 --- a/aptos-move/aptos-vm/src/aptos_vm_impl.rs +++ b/aptos-move/aptos-vm/src/aptos_vm_impl.rs @@ -22,8 +22,8 @@ use aptos_types::{ account_config::{TransactionValidation, APTOS_TRANSACTION_VALIDATION, CORE_CODE_ADDRESS}, chain_id::ChainId, on_chain_config::{ - ApprovedExecutionHashes, ConfigurationResource, FeatureFlag, Features, GasSchedule, - GasScheduleV2, OnChainConfig, StorageGasSchedule, TimedFeatures, Version, + ApprovedExecutionHashes, ConfigurationResource, Features, GasSchedule, GasScheduleV2, + OnChainConfig, StorageGasSchedule, TimedFeatures, Version, }, transaction::{AbortInfo, ExecutionStatus, Multisig, TransactionOutput, TransactionStatus}, vm_status::{StatusCode, VMStatus}, @@ -43,10 +43,9 @@ use std::sync::Arc; pub const MAXIMUM_APPROVED_TRANSACTION_SIZE: u64 = 1024 * 1024; -#[derive(Clone)] -/// A wrapper to make VMRuntime standalone and thread safe. +/// A wrapper to make VMRuntime standalone pub struct AptosVMImpl { - move_vm: Arc, + move_vm: MoveVmExt, gas_feature_version: u64, gas_params: Option, storage_gas_params: Option, @@ -152,7 +151,7 @@ impl AptosVMImpl { .expect("should be able to create Move VM; check if there are duplicated natives"); let mut vm = Self { - move_vm: Arc::new(inner), + move_vm: inner, gas_feature_version, gas_params, storage_gas_params, @@ -366,9 +365,9 @@ impl AptosVMImpl { /// Run the prologue of a transaction by calling into either `SCRIPT_PROLOGUE_NAME` function /// or `MULTI_AGENT_SCRIPT_PROLOGUE_NAME` function stored in the `ACCOUNT_MODULE` on chain. - pub(crate) fn run_script_prologue( + pub(crate) fn run_script_prologue( &self, - session: &mut SessionExt, + session: &mut SessionExt, txn_data: &TransactionMetadata, log_context: &AdapterLogSchema, ) -> Result<(), VMStatus> { @@ -430,9 +429,9 @@ impl AptosVMImpl { /// Run the prologue of a transaction by calling into `MODULE_PROLOGUE_NAME` function stored /// in the `ACCOUNT_MODULE` on chain. - pub(crate) fn run_module_prologue( + pub(crate) fn run_module_prologue( &self, - session: &mut SessionExt, + session: &mut SessionExt, txn_data: &TransactionMetadata, log_context: &AdapterLogSchema, ) -> Result<(), VMStatus> { @@ -472,9 +471,9 @@ impl AptosVMImpl { /// 2. It has received enough approvals to meet the signature threshold of the multisig account /// 3. If only the payload hash was stored on chain, the provided payload in execution should /// match that hash. - pub(crate) fn run_multisig_prologue( + pub(crate) fn run_multisig_prologue( &self, - session: &mut SessionExt, + session: &mut SessionExt, txn_data: &TransactionMetadata, payload: &Multisig, log_context: &AdapterLogSchema, @@ -507,9 +506,9 @@ impl AptosVMImpl { /// Run the epilogue of a transaction by calling into `EPILOGUE_NAME` function stored /// in the `ACCOUNT_MODULE` on chain. - pub(crate) fn run_success_epilogue( + pub(crate) fn run_success_epilogue( &self, - session: &mut SessionExt, + session: &mut SessionExt, gas_remaining: Gas, txn_data: &TransactionMetadata, log_context: &AdapterLogSchema, @@ -547,9 +546,9 @@ impl AptosVMImpl { /// Run the failure epilogue of a transaction by calling into `USER_EPILOGUE_NAME` function /// stored in the `ACCOUNT_MODULE` on chain. - pub(crate) fn run_failure_epilogue( + pub(crate) fn run_failure_epilogue( &self, - session: &mut SessionExt, + session: &mut SessionExt, gas_remaining: Gas, txn_data: &TransactionMetadata, log_context: &AdapterLogSchema, @@ -600,11 +599,7 @@ impl AptosVMImpl { &self, module: &ModuleId, ) -> Option { - if self.features.is_enabled(FeatureFlag::VM_BINARY_FORMAT_V6) { - aptos_framework::get_vm_metadata(&self.move_vm, module.clone()) - } else { - aptos_framework::get_vm_metadata_v0(&self.move_vm, module.clone()) - } + aptos_framework::get_vm_metadata(&self.move_vm, module.clone()) } pub fn new_move_resolver<'r, R: MoveResolverExt>( @@ -618,7 +613,7 @@ impl AptosVMImpl { &self, r: &'r R, session_id: SessionId, - ) -> SessionExt<'r, '_, R> { + ) -> SessionExt<'r, '_> { self.move_vm.new_session(r, session_id) } @@ -659,9 +654,9 @@ impl<'a> AptosVMInternals<'a> { } } -pub(crate) fn get_transaction_output( +pub(crate) fn get_transaction_output( ap_cache: &mut A, - session: SessionExt, + session: SessionExt, gas_left: Gas, txn_data: &TransactionMetadata, status: ExecutionStatus, @@ -672,9 +667,7 @@ pub(crate) fn get_transaction_output( .checked_sub(gas_left) .expect("Balance should always be less than or equal to max gas amount"); - let change_set_ext = session - .finish(ap_cache, change_set_configs) - .map_err(|e| e.into_vm_status())?; + let change_set_ext = session.finish(ap_cache, change_set_configs)?; let (delta_change_set, change_set) = change_set_ext.into_inner(); let (write_set, events) = change_set.into_inner(); diff --git a/aptos-move/aptos-vm/src/data_cache.rs b/aptos-move/aptos-vm/src/data_cache.rs index b369d535937b7..29438bcdcd528 100644 --- a/aptos-move/aptos-vm/src/data_cache.rs +++ b/aptos-move/aptos-vm/src/data_cache.rs @@ -22,7 +22,33 @@ use move_core_types::{ }; use move_table_extension::{TableHandle, TableResolver}; use move_vm_runtime::move_vm::MoveVM; -use std::ops::{Deref, DerefMut}; +use std::{ + collections::BTreeMap, + ops::{Deref, DerefMut}, +}; + +pub(crate) fn get_any_resource( + move_resolver: &impl MoveResolverExt, + address: &AccountAddress, + struct_tag: &StructTag, +) -> Result>, VMError> { + let resource_group = move_resolver.get_resource_group(struct_tag); + if let Some(resource_group) = resource_group { + let group_data = move_resolver.get_resource_group_data(address, &resource_group)?; + if let Some(group_data) = group_data { + let mut group_data: BTreeMap> = bcs::from_bytes(&group_data) + .map_err(|_| { + PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) + .finish(Location::Undefined) + })?; + Ok(group_data.remove(struct_tag)) + } else { + Ok(None) + } + } else { + move_resolver.get_standard_resource(address, struct_tag) + } +} pub struct MoveResolverWithVMMetadata<'a, 'm, S> { move_resolver: &'a S, @@ -74,7 +100,7 @@ impl<'a, 'm, S: MoveResolverExt> ResourceResolver for MoveResolverWithVMMetadata address: &AccountAddress, struct_tag: &StructTag, ) -> Result>, Error> { - Ok(self.get_any_resource(address, struct_tag)?) + Ok(get_any_resource(self, address, struct_tag)?) } } @@ -167,7 +193,7 @@ impl<'a, S: StateView> ResourceResolver for StorageAdapter<'a, S> { address: &AccountAddress, struct_tag: &StructTag, ) -> Result>, Error> { - Ok(self.get_any_resource(address, struct_tag)?) + Ok(get_any_resource(self, address, struct_tag)?) } } diff --git a/aptos-move/aptos-vm/src/move_vm_ext/resolver.rs b/aptos-move/aptos-vm/src/move_vm_ext/resolver.rs index 4728a7cbe2621..bb77d550c6f24 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/resolver.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/resolver.rs @@ -4,15 +4,25 @@ use aptos_framework::{natives::state_storage::StateStorageUsageResolver, RuntimeModuleMetadataV1}; use aptos_state_view::StateView; use aptos_types::on_chain_config::ConfigStorage; -use move_binary_format::errors::{Location, PartialVMError, VMError}; +use aptos_utils::{aptos_try, return_on_failure}; +use move_binary_format::errors::VMError; use move_core_types::{ account_address::AccountAddress, language_storage::{ModuleId, StructTag}, resolver::MoveResolver, - vm_status::StatusCode, }; use move_table_extension::TableResolver; -use std::collections::BTreeMap; + +fn get_resource_group_from_metadata( + struct_tag: &StructTag, + metadata: Option, +) -> Option { + metadata? + .struct_attributes + .get(struct_tag.name.as_ident_str().as_str())? + .iter() + .find_map(|attr| attr.get_resource_group_member()) +} pub trait MoveResolverExt: MoveResolver + TableResolver + StateStorageUsageResolver + ConfigStorage + StateView @@ -31,74 +41,22 @@ pub trait MoveResolverExt: struct_tag: &StructTag, ) -> Result>, VMError>; - fn get_any_resource( - &self, - address: &AccountAddress, - struct_tag: &StructTag, - ) -> Result>, VMError> { + fn get_resource_group(&self, struct_tag: &StructTag) -> Option { let metadata = self.get_module_metadata(struct_tag.module_id()); - let resource_group = Self::get_resource_group_from_metadata(struct_tag, metadata); - if let Some(resource_group) = resource_group { - self.get_resource_from_group(address, struct_tag, &resource_group) - } else { - self.get_standard_resource(address, struct_tag) - } - } - - fn get_resource_from_group( - &self, - address: &AccountAddress, - struct_tag: &StructTag, - resource_group: &StructTag, - ) -> Result>, VMError> { - let group_data = self.get_resource_group_data(address, resource_group)?; - if let Some(group_data) = group_data { - let mut group_data: BTreeMap> = bcs::from_bytes(&group_data) - .map_err(|_| { - PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) - .finish(Location::Undefined) - })?; - Ok(group_data.remove(struct_tag)) - } else { - Ok(None) - } + get_resource_group_from_metadata(struct_tag, metadata) } - fn get_resource_group(&self, struct_tag: &StructTag) -> Result, VMError> { - let metadata = self.get_module_metadata(struct_tag.module_id()); - Ok(Self::get_resource_group_from_metadata(struct_tag, metadata)) - } - - fn get_resource_group_from_metadata( - struct_tag: &StructTag, - metadata: Option, - ) -> Option { - metadata.and_then(|metadata| { - metadata + // Move to API does not belong here + fn is_resource_group(&self, struct_tag: &StructTag) -> bool { + aptos_try!({ + return_on_failure!(self + .get_module_metadata(struct_tag.module_id())? .struct_attributes - .get(struct_tag.name.as_ident_str().as_str()) - .and_then(|attrs| { - attrs - .iter() - .find_map(|attr| attr.get_resource_group_member()) - }) + .get(struct_tag.name.as_ident_str().as_str())? + .iter() + .find(|attr| attr.is_resource_group())); + Some(()) }) - } - - fn is_resource_group(&self, struct_tag: &StructTag) -> bool { - let metadata = self.get_module_metadata(struct_tag.module_id()); - metadata - .and_then(|metadata| { - metadata - .struct_attributes - .get(struct_tag.name.as_ident_str().as_str()) - .and_then(|attrs| { - attrs - .iter() - .map(|attr| Some(attr.is_resource_group())) - .next() - }) - }) - .is_some() + .is_some() } } diff --git a/aptos-move/aptos-vm/src/move_vm_ext/session.rs b/aptos-move/aptos-vm/src/move_vm_ext/session.rs index f250293039bdb..896b2177f1fa2 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/session.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/session.rs @@ -2,8 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - access_path_cache::AccessPathCache, data_cache::MoveResolverWithVMMetadata, - move_vm_ext::MoveResolverExt, transaction_metadata::TransactionMetadata, + access_path_cache::AccessPathCache, move_vm_ext::MoveResolverExt, + transaction_metadata::TransactionMetadata, }; use aptos_aggregator::{ aggregator_extension::AggregatorID, @@ -34,7 +34,7 @@ use move_core_types::{ vm_status::{StatusCode, VMStatus}, }; use move_table_extension::{NativeTableContext, TableChangeSet}; -use move_vm_runtime::{move_vm::MoveVM, session::Session}; +use move_vm_runtime::session::Session; use serde::{Deserialize, Serialize}; use std::{ collections::BTreeMap, @@ -93,20 +93,14 @@ impl SessionId { } } -pub struct SessionExt<'r, 'l, S> { +pub struct SessionExt<'r, 'l> { inner: Session<'r, 'l>, - remote: MoveResolverWithVMMetadata<'r, 'l, S>, + remote: &'r dyn MoveResolverExt, } -impl<'r, 'l, S> SessionExt<'r, 'l, S> -where - S: MoveResolverExt + 'r, -{ - pub fn new(inner: Session<'r, 'l>, move_vm: &'l MoveVM, remote: &'r S) -> Self { - Self { - inner, - remote: MoveResolverWithVMMetadata::new(remote, move_vm), - } +impl<'r, 'l> SessionExt<'r, 'l> { + pub fn new(inner: Session<'r, 'l>, remote: &'r dyn MoveResolverExt) -> Self { + Self { inner, remote } } pub fn finish( @@ -116,7 +110,7 @@ where ) -> VMResult { let (change_set, events, mut extensions) = self.inner.finish_with_extensions()?; let (change_set, resource_group_change_set) = - Self::split_and_merge_resource_groups(&self.remote, change_set)?; + Self::split_and_merge_resource_groups(self.remote, change_set)?; let table_context: NativeTableContext = extensions.remove(); let table_change_set = table_context @@ -162,7 +156,7 @@ where /// * If elements remain, Modify /// * Otherwise delete fn split_and_merge_resource_groups( - remote: &MoveResolverWithVMMetadata, + remote: &dyn MoveResolverExt, change_set: MoveChangeSet, ) -> VMResult<(MoveChangeSet, MoveChangeSet)> { // The use of this implies that we could theoretically call unwrap with no consequences, @@ -177,9 +171,7 @@ where let (modules, resources) = account_changeset.into_inner(); for (struct_tag, blob_op) in resources { - let resource_group = remote - .get_resource_group(&struct_tag) - .map_err(|_| common_error.clone())?; + let resource_group = remote.get_resource_group(&struct_tag); if let Some(resource_group) = resource_group { resource_groups .entry(resource_group) @@ -362,7 +354,7 @@ where } } -impl<'r, 'l, S> Deref for SessionExt<'r, 'l, S> { +impl<'r, 'l> Deref for SessionExt<'r, 'l> { type Target = Session<'r, 'l>; fn deref(&self) -> &Self::Target { @@ -370,7 +362,7 @@ impl<'r, 'l, S> Deref for SessionExt<'r, 'l, S> { } } -impl<'r, 'l, S> DerefMut for SessionExt<'r, 'l, S> { +impl<'r, 'l> DerefMut for SessionExt<'r, 'l> { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.inner } diff --git a/aptos-move/aptos-vm/src/move_vm_ext/vm.rs b/aptos-move/aptos-vm/src/move_vm_ext/vm.rs index 337ba645c3025..a1d88b6abe07a 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/vm.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/vm.rs @@ -77,7 +77,7 @@ impl MoveVmExt { &self, remote: &'r S, session_id: SessionId, - ) -> SessionExt<'r, '_, S> { + ) -> SessionExt<'r, '_> { let mut extensions = NativeContextExtensions::default(); let txn_hash: [u8; 32] = session_id .as_uuid() @@ -109,7 +109,6 @@ impl MoveVmExt { SessionExt::new( self.inner.new_session_with_extensions(remote, extensions), - self, remote, ) } diff --git a/aptos-move/aptos-vm/src/verifier/resource_groups.rs b/aptos-move/aptos-vm/src/verifier/resource_groups.rs index 2f0d98af67f34..6311282c11820 100644 --- a/aptos-move/aptos-vm/src/verifier/resource_groups.rs +++ b/aptos-move/aptos-vm/src/verifier/resource_groups.rs @@ -1,7 +1,7 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 -use crate::move_vm_ext::{MoveResolverExt, SessionExt}; +use crate::move_vm_ext::SessionExt; use aptos_framework::{ResourceGroupScope, RuntimeModuleMetadataV1}; use move_binary_format::{ errors::{Location, PartialVMError, VMError, VMResult}, @@ -29,8 +29,8 @@ fn metadata_validation_error(msg: &str) -> VMError { /// * Ensure that each member has a membership and it does not change /// * Ensure that each group has a scope and that it does not become more restrictive /// * For any new members, verify that they are in a valid resource group -pub(crate) fn validate_resource_groups( - session: &mut SessionExt, +pub(crate) fn validate_resource_groups( + session: &mut SessionExt, modules: &[CompiledModule], ) -> Result<(), VMError> { let mut groups = BTreeMap::new(); @@ -72,8 +72,8 @@ pub(crate) fn validate_resource_groups( /// * Extract the resource group metadata /// * Verify all changes are compatible upgrades /// * Return any new members to validate correctness and all groups to assist in validation -pub(crate) fn validate_module_and_extract_new_entries( - session: &mut SessionExt, +pub(crate) fn validate_module_and_extract_new_entries( + session: &mut SessionExt, module: &CompiledModule, ) -> VMResult<( BTreeMap, @@ -111,8 +111,8 @@ pub(crate) fn validate_module_and_extract_new_entries( } /// Given a module id extract all resource group metadata -pub(crate) fn extract_resource_group_metadata_from_module( - session: &mut SessionExt, +pub(crate) fn extract_resource_group_metadata_from_module( + session: &mut SessionExt, module_id: &ModuleId, ) -> VMResult<( BTreeMap, diff --git a/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs b/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs index 448f06e58d799..3e69987cd8ac9 100644 --- a/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs +++ b/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs @@ -6,10 +6,7 @@ //! TODO: we should not only validate the types but also the actual values, e.g. //! for strings whether they consist of correct characters. -use crate::{ - move_vm_ext::{MoveResolverExt, SessionExt}, - VMStatus, -}; +use crate::{move_vm_ext::SessionExt, VMStatus}; use move_binary_format::{errors::VMError, file_format_common::read_uleb128_as_u64}; use move_core_types::{ account_address::AccountAddress, @@ -97,8 +94,8 @@ pub(crate) fn get_allowed_structs( /// 3. check arg types are allowed after signers /// /// after validation, add senders and non-signer arguments to generate the final args -pub(crate) fn validate_combine_signer_and_txn_args( - session: &mut SessionExt, +pub(crate) fn validate_combine_signer_and_txn_args( + session: &mut SessionExt, senders: Vec, args: Vec>, func: &LoadedFunctionInstantiation, @@ -183,8 +180,8 @@ pub(crate) fn validate_combine_signer_and_txn_args( } // Return whether the argument is valid/allowed and whether it needs construction. -pub(crate) fn is_valid_txn_arg( - session: &SessionExt, +pub(crate) fn is_valid_txn_arg( + session: &SessionExt, typ: &Type, allowed_structs: &ConstructorMap, ) -> (bool, bool) { @@ -208,8 +205,8 @@ pub(crate) fn is_valid_txn_arg( // Construct arguments. Walk through the arguments and according to the signature // construct arguments that require so. // TODO: This needs a more solid story and a tighter integration with the VM. -pub(crate) fn construct_args( - session: &mut SessionExt, +pub(crate) fn construct_args( + session: &mut SessionExt, idxs: &[usize], args: &mut [Vec], func: &LoadedFunctionInstantiation, @@ -250,8 +247,8 @@ pub(crate) fn construct_args( // A Cursor is used to recursively walk the serialized arg manually and correctly. In effect we // are parsing the BCS serialized implicit constructor invocation tree, while serializing the // constructed types into the output parameter arg. -pub(crate) fn recursively_construct_arg( - session: &mut SessionExt, +pub(crate) fn recursively_construct_arg( + session: &mut SessionExt, ty: &Type, allowed_structs: &ConstructorMap, cursor: &mut Cursor<&[u8]>, @@ -309,8 +306,8 @@ pub(crate) fn recursively_construct_arg( // constructed value. This is the correct data to pass as the argument to a function taking // said struct as a parameter. In this function we execute the constructor constructing the // value and returning the BCS serialized representation. -fn validate_and_construct( - session: &mut SessionExt, +fn validate_and_construct( + session: &mut SessionExt, expected_type: &Type, constructor: &FunctionId, allowed_structs: &ConstructorMap, diff --git a/aptos-move/aptos-vm/src/verifier/view_function.rs b/aptos-move/aptos-vm/src/verifier/view_function.rs index 9921b84982b9a..1870fddcf9ddd 100644 --- a/aptos-move/aptos-vm/src/verifier/view_function.rs +++ b/aptos-move/aptos-vm/src/verifier/view_function.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - move_vm_ext::{MoveResolverExt, SessionExt}, + move_vm_ext::SessionExt, verifier::{transaction_arg_validation, transaction_arg_validation::get_allowed_structs}, }; use aptos_framework::RuntimeModuleMetadataV1; @@ -29,8 +29,8 @@ pub fn determine_is_view( /// Validate view function call. This checks whether the function is marked as a view /// function, and validates the arguments. -pub(crate) fn validate_view_function( - session: &mut SessionExt, +pub(crate) fn validate_view_function( + session: &mut SessionExt, mut args: Vec>, fun_name: &IdentStr, fun_inst: &LoadedFunctionInstantiation, diff --git a/aptos-move/framework/src/built_package.rs b/aptos-move/framework/src/built_package.rs index 87434990c26a0..b7f2c035bbd20 100644 --- a/aptos-move/framework/src/built_package.rs +++ b/aptos-move/framework/src/built_package.rs @@ -362,7 +362,7 @@ fn inject_runtime_metadata( let serialized_metadata = bcs::to_bytes(&module_metadata) .expect("BCS for RuntimeModuleMetadata"); named_module.module.metadata.push(Metadata { - key: APTOS_METADATA_KEY_V1.clone(), + key: APTOS_METADATA_KEY_V1.to_vec(), value: serialized_metadata, }); } else { @@ -370,7 +370,7 @@ fn inject_runtime_metadata( bcs::to_bytes(&module_metadata.clone().downgrade()) .expect("BCS for RuntimeModuleMetadata"); named_module.module.metadata.push(Metadata { - key: APTOS_METADATA_KEY.clone(), + key: APTOS_METADATA_KEY.to_vec(), value: serialized_metadata, }); } diff --git a/aptos-move/framework/src/module_metadata.rs b/aptos-move/framework/src/module_metadata.rs index 815e6f518893e..b62162477c73f 100644 --- a/aptos-move/framework/src/module_metadata.rs +++ b/aptos-move/framework/src/module_metadata.rs @@ -15,7 +15,6 @@ use move_core_types::{ metadata::Metadata, }; use move_vm_runtime::move_vm::MoveVM; -use once_cell::sync::Lazy; use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; use thiserror::Error; @@ -26,10 +25,8 @@ pub const METADATA_V1_MIN_FILE_FORMAT_VERSION: u32 = 6; /// The keys used to identify the metadata in the metadata section of the module bytecode. /// This is more or less arbitrary, besides we should use some unique key to identify /// Aptos specific metadata (`aptos::` here). -pub static APTOS_METADATA_KEY: Lazy> = - Lazy::new(|| "aptos::metadata_v0".as_bytes().to_vec()); -pub static APTOS_METADATA_KEY_V1: Lazy> = - Lazy::new(|| "aptos::metadata_v1".as_bytes().to_vec()); +pub static APTOS_METADATA_KEY: &[u8] = "aptos::metadata_v0".as_bytes(); +pub static APTOS_METADATA_KEY_V1: &[u8] = "aptos::metadata_v1".as_bytes(); /// Aptos specific metadata attached to the metadata section of file_format. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -112,7 +109,7 @@ impl KnownAttribute { pub fn get_resource_group_member(&self) -> Option { if self.kind == KnownAttributeKind::ResourceGroupMember as u8 { - self.args.get(0).and_then(|group| str::parse(group).ok()) + self.args.get(0)?.parse().ok() } else { None } @@ -125,20 +122,16 @@ impl KnownAttribute { /// Extract metadata from the VM, upgrading V0 to V1 representation as needed pub fn get_vm_metadata(vm: &MoveVM, module_id: ModuleId) -> Option { - if let Some(data) = vm.get_module_metadata(module_id.clone(), &APTOS_METADATA_KEY_V1) { + if let Some(data) = vm.get_module_metadata(module_id.clone(), APTOS_METADATA_KEY_V1) { bcs::from_bytes::(&data.value).ok() - } else if let Some(data) = vm.get_module_metadata(module_id, &APTOS_METADATA_KEY) { - // Old format available, upgrade to new one on the fly - let data_v0 = bcs::from_bytes::(&data.value).ok()?; - Some(data_v0.upgrade()) } else { - None + get_vm_metadata_v0(vm, module_id) } } /// Extract metadata from the VM, legacy V0 format upgraded to V1 -pub fn get_vm_metadata_v0(vm: &MoveVM, module_id: ModuleId) -> Option { - if let Some(data) = vm.get_module_metadata(module_id, &APTOS_METADATA_KEY) { +fn get_vm_metadata_v0(vm: &MoveVM, module_id: ModuleId) -> Option { + if let Some(data) = vm.get_module_metadata(module_id, APTOS_METADATA_KEY) { let data_v0 = bcs::from_bytes::(&data.value).ok()?; Some(data_v0.upgrade()) } else { @@ -175,7 +168,7 @@ pub fn check_metadata_format(module: &CompiledModule) -> Result<(), MalformedErr pub fn get_metadata_from_compiled_module( module: &CompiledModule, ) -> Option { - if let Some(data) = find_metadata(module, &APTOS_METADATA_KEY_V1) { + if let Some(data) = find_metadata(module, APTOS_METADATA_KEY_V1) { let mut metadata = bcs::from_bytes::(&data.value).ok(); // Clear out metadata for v5, since it shouldn't have existed in the first place and isn't // being used. Note, this should have been gated in the verify module metadata. @@ -186,7 +179,7 @@ pub fn get_metadata_from_compiled_module( } } metadata - } else if let Some(data) = find_metadata(module, &APTOS_METADATA_KEY) { + } else if let Some(data) = find_metadata(module, APTOS_METADATA_KEY) { // Old format available, upgrade to new one on the fly let data_v0 = bcs::from_bytes::(&data.value).ok()?; Some(data_v0.upgrade()) @@ -208,7 +201,7 @@ pub fn get_metadata_from_compiled_module( pub fn get_metadata_from_compiled_script( script: &CompiledScript, ) -> Option { - if let Some(data) = find_metadata_in_script(script, &APTOS_METADATA_KEY_V1) { + if let Some(data) = find_metadata_in_script(script, APTOS_METADATA_KEY_V1) { let mut metadata = bcs::from_bytes::(&data.value).ok(); // Clear out metadata for v5, since it shouldn't have existed in the first place and isn't // being used. Note, this should have been gated in the verify module metadata. @@ -219,7 +212,7 @@ pub fn get_metadata_from_compiled_script( } } metadata - } else if let Some(data) = find_metadata_in_script(script, &APTOS_METADATA_KEY) { + } else if let Some(data) = find_metadata_in_script(script, APTOS_METADATA_KEY) { // Old format available, upgrade to new one on the fly let data_v0 = bcs::from_bytes::(&data.value).ok()?; Some(data_v0.upgrade()) diff --git a/aptos-move/vm-genesis/src/lib.rs b/aptos-move/vm-genesis/src/lib.rs index 8a70048b84cdd..d02a861903e59 100644 --- a/aptos-move/vm-genesis/src/lib.rs +++ b/aptos-move/vm-genesis/src/lib.rs @@ -35,7 +35,6 @@ use move_core_types::{ account_address::AccountAddress, identifier::Identifier, language_storage::{ModuleId, TypeTag}, - resolver::MoveResolver, value::{serialize_values, MoveValue}, }; use move_vm_types::gas::UnmeteredGasMeter; @@ -338,7 +337,7 @@ fn validate_genesis_config(genesis_config: &GenesisConfiguration) { } fn exec_function( - session: &mut SessionExt, + session: &mut SessionExt, module_name: &str, function_name: &str, ty_args: Vec, @@ -367,7 +366,7 @@ fn exec_function( } fn initialize( - session: &mut SessionExt, + session: &mut SessionExt, chain_id: ChainId, genesis_config: &GenesisConfiguration, consensus_config: &OnChainConsensusConfig, @@ -437,7 +436,7 @@ pub fn default_features() -> Vec { ] } -fn initialize_features(session: &mut SessionExt) { +fn initialize_features(session: &mut SessionExt) { let features: Vec = default_features() .into_iter() .map(|feature| feature as u64) @@ -456,7 +455,7 @@ fn initialize_features(session: &mut SessionExt) { ); } -fn initialize_aptos_coin(session: &mut SessionExt) { +fn initialize_aptos_coin(session: &mut SessionExt) { exec_function( session, GENESIS_MODULE_NAME, @@ -466,7 +465,7 @@ fn initialize_aptos_coin(session: &mut SessionExt) { ); } -fn set_genesis_end(session: &mut SessionExt) { +fn set_genesis_end(session: &mut SessionExt) { exec_function( session, GENESIS_MODULE_NAME, @@ -477,7 +476,7 @@ fn set_genesis_end(session: &mut SessionExt) { } fn initialize_core_resources_and_aptos_coin( - session: &mut SessionExt, + session: &mut SessionExt, core_resources_key: &Ed25519PublicKey, ) { let core_resources_auth_key = AuthenticationKey::ed25519(core_resources_key); @@ -494,10 +493,7 @@ fn initialize_core_resources_and_aptos_coin( } /// Create and initialize Association and Core Code accounts. -fn initialize_on_chain_governance( - session: &mut SessionExt, - genesis_config: &GenesisConfiguration, -) { +fn initialize_on_chain_governance(session: &mut SessionExt, genesis_config: &GenesisConfiguration) { exec_function( session, GOVERNANCE_MODULE_NAME, @@ -512,7 +508,7 @@ fn initialize_on_chain_governance( ); } -fn create_accounts(session: &mut SessionExt, accounts: &[AccountBalance]) { +fn create_accounts(session: &mut SessionExt, accounts: &[AccountBalance]) { let accounts_bytes = bcs::to_bytes(accounts).expect("AccountMaps can be serialized"); let mut serialized_values = serialize_values(&vec![MoveValue::Signer(CORE_CODE_ADDRESS)]); serialized_values.push(accounts_bytes); @@ -526,7 +522,7 @@ fn create_accounts(session: &mut SessionExt, accounts: &[Acco } fn create_employee_validators( - session: &mut SessionExt, + session: &mut SessionExt, employees: &[EmployeePool], genesis_config: &GenesisConfiguration, ) { @@ -549,10 +545,7 @@ fn create_employee_validators( /// Creates and initializes each validator owner and validator operator. This method creates all /// the required accounts, sets the validator operators for each validator owner, and sets the /// validator config on-chain. -fn create_and_initialize_validators( - session: &mut SessionExt, - validators: &[Validator], -) { +fn create_and_initialize_validators(session: &mut SessionExt, validators: &[Validator]) { let validators_bytes = bcs::to_bytes(validators).expect("Validators can be serialized"); let mut serialized_values = serialize_values(&vec![MoveValue::Signer(CORE_CODE_ADDRESS)]); serialized_values.push(validators_bytes); @@ -566,7 +559,7 @@ fn create_and_initialize_validators( } fn create_and_initialize_validators_with_commission( - session: &mut SessionExt, + session: &mut SessionExt, validators: &[ValidatorWithCommissionRate], ) { let validators_bytes = bcs::to_bytes(validators).expect("Validators can be serialized"); @@ -584,7 +577,7 @@ fn create_and_initialize_validators_with_commission( ); } -fn allow_core_resources_to_set_version(session: &mut SessionExt) { +fn allow_core_resources_to_set_version(session: &mut SessionExt) { exec_function( session, VERSION_MODULE_NAME, @@ -595,14 +588,14 @@ fn allow_core_resources_to_set_version(session: &mut SessionExt, framework: &ReleaseBundle) { +fn publish_framework(session: &mut SessionExt, framework: &ReleaseBundle) { for pack in &framework.packages { publish_package(session, pack) } } /// Publish the given package. -fn publish_package(session: &mut SessionExt, pack: &ReleasePackage) { +fn publish_package(session: &mut SessionExt, pack: &ReleasePackage) { let modules = pack.sorted_code_and_modules(); let addr = *modules.first().unwrap().1.self_id().address(); let code = modules @@ -630,7 +623,7 @@ fn publish_package(session: &mut SessionExt, pack: &ReleasePa } /// Trigger a reconfiguration. This emits an event that will be passed along to the storage layer. -fn emit_new_block_and_epoch_event(session: &mut SessionExt) { +fn emit_new_block_and_epoch_event(session: &mut SessionExt) { exec_function( session, "block", diff --git a/aptos-move/writeset-transaction-generator/src/writeset_builder.rs b/aptos-move/writeset-transaction-generator/src/writeset_builder.rs index 8cc033fb1936a..a072fed89e7d4 100644 --- a/aptos-move/writeset-transaction-generator/src/writeset_builder.rs +++ b/aptos-move/writeset-transaction-generator/src/writeset_builder.rs @@ -17,7 +17,7 @@ use aptos_types::{ }; use aptos_vm::{ data_cache::StorageAdapter, - move_vm_ext::{MoveResolverExt, MoveVmExt, SessionExt, SessionId}, + move_vm_ext::{MoveVmExt, SessionExt, SessionId}, }; use move_core_types::{ identifier::Identifier, @@ -28,9 +28,9 @@ use move_core_types::{ use move_vm_runtime::session::SerializedReturnValues; use move_vm_types::gas::UnmeteredGasMeter; -pub struct GenesisSession<'r, 'l, S>(SessionExt<'r, 'l, S>); +pub struct GenesisSession<'r, 'l>(SessionExt<'r, 'l>); -impl<'r, 'l, S: MoveResolverExt> GenesisSession<'r, 'l, S> { +impl<'r, 'l> GenesisSession<'r, 'l> { pub fn exec_func( &mut self, module_name: &str, @@ -109,7 +109,7 @@ impl<'r, 'l, S: MoveResolverExt> GenesisSession<'r, 'l, S> { pub fn build_changeset(state_view: &S, procedure: F, chain_id: u8) -> ChangeSet where - F: FnOnce(&mut GenesisSession>), + F: FnOnce(&mut GenesisSession), { let move_vm = MoveVmExt::new( NativeGasParameters::zeros(), diff --git a/aptos-utils/Cargo.toml b/aptos-utils/Cargo.toml new file mode 100644 index 0000000000000..975e52bf4aedf --- /dev/null +++ b/aptos-utils/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "aptos-utils" +version = "0.1.0" +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +# Workspace inherited keys +authors = { workspace = true } +edition = { workspace = true } +homepage = { workspace = true } +license = { workspace = true } +publish = { workspace = true } +repository = { workspace = true } +rust-version = { workspace = true } + +[dependencies] diff --git a/aptos-utils/src/lib.rs b/aptos-utils/src/lib.rs new file mode 100644 index 0000000000000..4f5e122417184 --- /dev/null +++ b/aptos-utils/src/lib.rs @@ -0,0 +1,24 @@ +// Copyright © Aptos Foundation + +/// An internal implementation to imitate the feature of `try` in unstable Rust. +/// Useful to use '?' chaining on option/result without the need to wrap the expression in a +/// function. +/// Obsolete once rust-lang/rust#31436 is resolved and try is stable. +#[macro_export] +macro_rules! aptos_try { + ($e:expr) => { + (|| $e)() + }; +} + +/// When the expression is an error, return from the enclosing function. +/// Use this in cases where the enclosing function returns `Result<(), Error>`. +/// Writing 'f()?;' relies on a load-bearing '?' operator. If the operator is removed execution +/// would continue on error. This macro is a more explicit way to return on error, ie. +/// 'return_on_failure!(f());' +#[macro_export] +macro_rules! return_on_failure { + ($e:expr) => { + $e?; + }; +} diff --git a/third_party/move/move-vm/runtime/src/loader.rs b/third_party/move/move-vm/runtime/src/loader.rs index 3e194cc79f1ac..5e954eec78f57 100644 --- a/third_party/move/move-vm/runtime/src/loader.rs +++ b/third_party/move/move-vm/runtime/src/loader.rs @@ -72,7 +72,8 @@ where } fn get(&self, key: &K) -> Option<&Arc> { - self.id_map.get(key).and_then(|idx| self.binaries.get(*idx)) + let index = self.id_map.get(key)?; + self.binaries.get(*index) } } @@ -580,8 +581,11 @@ impl Loader { let cache = self.module_cache.read(); cache .modules - .get(&module) - .and_then(|module| module.module.metadata.iter().find(|md| md.key == key)) + .get(&module)? + .module + .metadata + .iter() + .find(|md| md.key == key) .cloned() }