[move] Minor improvements for code storage (#13892)

- `ModulePath` uses a boolean + have a new method to construct a state key from address + module name. 
- `DeltaStorage` removed.
- Move resolvers to VM types.
- `ResourceResolver` and `ModuleResolver` are clearly separated.

aptos-move/aptos-vm/src/aptos_vm.rs

@@ -2704,8 +2704,13 @@ pub(crate) fn is_account_init_for_sponsored_transaction(
             && txn_data.fee_payer.is_some()
             && txn_data.sequence_number == 0
             && resolver
-                .get_resource(&txn_data.sender(), &AccountResource::struct_tag())
-                .map(|data| data.is_none())
+                .get_resource_bytes_with_metadata_and_layout(
+                    &txn_data.sender(),
+                    &AccountResource::struct_tag(),
+                    &resolver.get_module_metadata(&AccountResource::struct_tag().module_id()),
+                    None,
+                )
+                .map(|(data, _)| data.is_none())
                 .map_err(|e| e.finish(Location::Undefined))?,
     )
 }

aptos-move/aptos-vm/src/data_cache.rs

@@ -40,10 +40,12 @@ use move_core_types::{
     account_address::AccountAddress,
     language_storage::{ModuleId, StructTag},
     metadata::Metadata,
-    resolver::{resource_size, ModuleResolver, ResourceResolver},
     value::MoveTypeLayout,
 };
-use move_vm_types::delayed_values::delayed_field_id::DelayedFieldID;
+use move_vm_types::{
+    delayed_values::delayed_field_id::DelayedFieldID,
+    resolver::{resource_size, ModuleResolver, ResourceResolver},
+};
 use std::{
     cell::RefCell,
     collections::{BTreeMap, HashMap, HashSet},
@@ -172,22 +174,18 @@ impl<'e, E: ExecutorView> ResourceGroupResolver for StorageAdapter<'e, E> {
 impl<'e, E: ExecutorView> AptosMoveResolver for StorageAdapter<'e, E> {}
 
 impl<'e, E: ExecutorView> ResourceResolver for StorageAdapter<'e, E> {
-    type Error = PartialVMError;
-
     fn get_resource_bytes_with_metadata_and_layout(
         &self,
         address: &AccountAddress,
         struct_tag: &StructTag,
         metadata: &[Metadata],
         maybe_layout: Option<&MoveTypeLayout>,
-    ) -> Result<(Option<Bytes>, usize), Self::Error> {
+    ) -> PartialVMResult<(Option<Bytes>, usize)> {
         self.get_any_resource_with_layout(address, struct_tag, metadata, maybe_layout)
     }
 }
 
 impl<'e, E: ExecutorView> ModuleResolver for StorageAdapter<'e, E> {
-    type Error = PartialVMError;
-
     fn get_module_metadata(&self, module_id: &ModuleId) -> Vec<Metadata> {
         let module_bytes = match self.get_module(module_id) {
             Ok(Some(bytes)) => bytes,
@@ -202,7 +200,7 @@ impl<'e, E: ExecutorView> ModuleResolver for StorageAdapter<'e, E> {
         module.metadata
     }
 
-    fn get_module(&self, module_id: &ModuleId) -> Result<Option<Bytes>, Self::Error> {
+    fn get_module(&self, module_id: &ModuleId) -> PartialVMResult<Option<Bytes>> {
         self.executor_view
             .get_module_bytes(&StateKey::module_id(module_id))
     }

aptos-move/aptos-vm/src/keyless_validation.rs

@@ -33,9 +33,16 @@ macro_rules! value_deserialization_error {
 fn get_resource_on_chain<T: MoveStructType + for<'a> Deserialize<'a>>(
     resolver: &impl AptosMoveResolver,
 ) -> anyhow::Result<T, VMStatus> {
+    let metadata = resolver.get_module_metadata(&T::struct_tag().module_id());
     let bytes = resolver
-        .get_resource(&CORE_CODE_ADDRESS, &T::struct_tag())
+        .get_resource_bytes_with_metadata_and_layout(
+            &CORE_CODE_ADDRESS,
+            &T::struct_tag(),
+            &metadata,
+            None,
+        )
         .map_err(|e| e.finish(Location::Undefined).into_vm_status())?
+        .0
         .ok_or_else(|| {
             value_deserialization_error!(format!(
                 "get_resource failed on {}::{}::{}",

aptos-move/aptos-vm/src/move_vm_ext/resolver.rs

@@ -8,8 +8,9 @@ use aptos_vm_types::resolver::{
     ExecutorView, ResourceGroupSize, ResourceGroupView, StateStorageView,
 };
 use bytes::Bytes;
-use move_binary_format::errors::{PartialVMError, PartialVMResult};
-use move_core_types::{language_storage::StructTag, resolver::MoveResolver};
+use move_binary_format::errors::PartialVMResult;
+use move_core_types::language_storage::StructTag;
+use move_vm_types::resolver::{ModuleResolver, ResourceResolver};
 use std::collections::{BTreeMap, HashMap};
 
 /// A general resolver used by AptosVM. Allows to implement custom hooks on
@@ -19,7 +20,8 @@ pub trait AptosMoveResolver:
     AggregatorV1Resolver
     + ConfigStorage
     + DelayedFieldResolver
-    + MoveResolver<PartialVMError>
+    + ModuleResolver
+    + ResourceResolver
     + ResourceGroupResolver
     + StateStorageView
     + TableResolver

aptos-move/block-executor/src/proptest_types/types.rs

@@ -10,7 +10,6 @@ use aptos_aggregator::{
 };
 use aptos_mvhashmap::types::TxnIndex;
 use aptos_types::{
-    access_path::AccessPath,
     account_address::AccountAddress,
     contract_event::TransactionEvent,
     delayed_fields::PanicError,
@@ -29,14 +28,13 @@ use aptos_types::{
 use aptos_vm_types::resolver::{TExecutorView, TResourceGroupView};
 use bytes::Bytes;
 use claims::{assert_ge, assert_le, assert_ok};
-use move_core_types::value::MoveTypeLayout;
+use move_core_types::{identifier::IdentStr, value::MoveTypeLayout};
 use move_vm_types::delayed_values::delayed_field_id::DelayedFieldID;
 use once_cell::sync::OnceCell;
 use proptest::{arbitrary::Arbitrary, collection::vec, prelude::*, proptest, sample::Index};
 use proptest_derive::Arbitrary;
 use std::{
     collections::{hash_map::DefaultHasher, BTreeMap, BTreeSet, HashMap, HashSet},
-    convert::TryInto,
     fmt::Debug,
     hash::{Hash, Hasher},
     marker::PhantomData,
@@ -154,21 +152,12 @@ pub(crate) struct KeyType<K: Hash + Clone + Debug + PartialOrd + Ord + Eq>(
 );
 
 impl<K: Hash + Clone + Debug + Eq + PartialOrd + Ord> ModulePath for KeyType<K> {
-    fn module_path(&self) -> Option<AccessPath> {
-        // Since K is generic, use its hash to assign addresses.
-        let mut hasher = DefaultHasher::new();
-        self.0.hash(&mut hasher);
-        let mut hashed_address = vec![1u8; AccountAddress::LENGTH - 8];
-        hashed_address.extend_from_slice(&hasher.finish().to_ne_bytes());
-
-        if self.1 {
-            Some(AccessPath {
-                address: AccountAddress::new(hashed_address.try_into().unwrap()),
-                path: b"/foo/b".to_vec(),
-            })
-        } else {
-            None
-        }
+    fn is_module_path(&self) -> bool {
+        self.1
+    }
+
+    fn from_address_and_module_name(_address: &AccountAddress, _module_name: &IdentStr) -> Self {
+        unimplemented!()
     }
 }
 
@@ -894,15 +883,16 @@ where
                 for k in behavior.reads.iter() {
                     // TODO: later test errors as well? (by fixing state_view behavior).
                     // TODO: test aggregator reads.
-                    match k.module_path() {
-                        Some(_) => match view.get_module_bytes(k) {
+                    if k.is_module_path() {
+                        match view.get_module_bytes(k) {
                             Ok(v) => read_results.push(v.map(Into::into)),
                             Err(_) => read_results.push(None),
-                        },
-                        None => match view.get_resource_bytes(k, None) {
+                        }
+                    } else {
+                        match view.get_resource_bytes(k, None) {
                             Ok(v) => read_results.push(v.map(Into::into)),
                             Err(_) => read_results.push(None),
-                        },
+                        }
                     }
                 }
                 // Read from groups.
@@ -1057,7 +1047,7 @@ where
     fn resource_write_set(&self) -> Vec<(K, Arc<ValueType>, Option<Arc<MoveTypeLayout>>)> {
         self.writes
             .iter()
-            .filter(|(k, _)| k.module_path().is_none())
+            .filter(|(k, _)| !k.is_module_path())
             .cloned()
             .map(|(k, v)| (k, Arc::new(v), None))
             .collect()
@@ -1066,7 +1056,7 @@ where
     fn module_write_set(&self) -> BTreeMap<K, ValueType> {
         self.writes
             .iter()
-            .filter(|(k, _)| k.module_path().is_some())
+            .filter(|(k, _)| k.is_module_path())
             .cloned()
             .collect()
     }

aptos-move/block-executor/src/view.rs

@@ -1296,7 +1296,7 @@ impl<'a, T: Transaction, S: TStateView<Key = T::Key>, X: Executable> LatestView<
         kind: ReadKind,
     ) -> PartialVMResult<ReadResult> {
         debug_assert!(
-            state_key.module_path().is_none(),
+            !state_key.is_module_path(),
             "Reading a module {:?} using ResourceView",
             state_key,
         );
@@ -1529,7 +1529,7 @@ impl<'a, T: Transaction, S: TStateView<Key = T::Key>, X: Executable> TModuleView
 
     fn get_module_state_value(&self, state_key: &Self::Key) -> PartialVMResult<Option<StateValue>> {
         debug_assert!(
-            state_key.module_path().is_some(),
+            state_key.is_module_path(),
             "Reading a resource {:?} using ModuleView",
             state_key,
         );

aptos-move/mvhashmap/src/types.rs

@@ -246,13 +246,13 @@ pub(crate) mod test {
     use super::*;
     use aptos_aggregator::delta_change_set::serialize;
     use aptos_types::{
-        access_path::AccessPath,
         executable::ModulePath,
         state_store::state_value::StateValue,
         write_set::{TransactionWrite, WriteOpKind},
     };
     use bytes::Bytes;
     use claims::{assert_err, assert_ok_eq};
+    use move_core_types::{account_address::AccountAddress, identifier::IdentStr};
     use std::{fmt::Debug, hash::Hash, sync::Arc};
 
     #[derive(Clone, Eq, Hash, PartialEq, Debug)]
@@ -262,8 +262,15 @@ pub(crate) mod test {
     );
 
     impl<K: Hash + Clone + Eq + Debug> ModulePath for KeyType<K> {
-        fn module_path(&self) -> Option<AccessPath> {
-            None
+        fn is_module_path(&self) -> bool {
+            false
+        }
+
+        fn from_address_and_module_name(
+            _address: &AccountAddress,
+            _module_name: &IdentStr,
+        ) -> Self {
+            unreachable!("Irrelevant for test")
         }
     }
 

aptos-move/mvhashmap/src/unsync_map.rs

@@ -242,7 +242,7 @@ impl<
 
     pub fn fetch_module(&self, key: &K) -> Option<MVModulesOutput<V, X>> {
         use MVModulesOutput::*;
-        debug_assert!(key.module_path().is_some());
+        debug_assert!(key.is_module_path());
 
         self.module_map.borrow_mut().get_mut(key).map(|entry| {
             let hash = entry.1.get_or_insert(module_hash(entry.0.as_ref()));

third_party/move/extensions/async/move-async-vm/src/async_vm.rs

@@ -14,7 +14,6 @@ use move_core_types::{
     effects::{ChangeSet, Op},
     identifier::Identifier,
     language_storage::{ModuleId, StructTag, TypeTag},
-    resolver::MoveResolver,
     vm_status::StatusCode,
 };
 use move_vm_runtime::{
@@ -25,7 +24,10 @@ use move_vm_runtime::{
     session::{SerializedReturnValues, Session},
 };
 use move_vm_test_utils::gas_schedule::{Gas, GasStatus};
-use move_vm_types::values::{Reference, Value};
+use move_vm_types::{
+    resolver::MoveResolver,
+    values::{Reference, Value},
+};
 use std::{
     collections::HashMap,
     error::Error,
@@ -82,7 +84,7 @@ impl AsyncVM {
         &'l self,
         for_actor: AccountAddress,
         virtual_time: u128,
-        move_resolver: &'r mut impl MoveResolver<PartialVMError>,
+        move_resolver: &'r impl MoveResolver,
     ) -> AsyncSession<'r, 'l> {
         self.new_session_with_extensions(
             for_actor,
@@ -97,7 +99,7 @@ impl AsyncVM {
         &'l self,
         for_actor: AccountAddress,
         virtual_time: u128,
-        move_resolver: &'r mut impl MoveResolver<PartialVMError>,
+        move_resolver: &'r impl MoveResolver,
         ext: NativeContextExtensions<'r>,
     ) -> AsyncSession<'r, 'l> {
         let extensions = make_extensions(ext, for_actor, virtual_time, true);

third_party/move/extensions/async/move-async-vm/tests/testsuite.rs

@@ -10,7 +10,7 @@ use move_async_vm::{
     async_vm::{AsyncResult, AsyncSession, AsyncVM, Message},
     natives::GasParameters as ActorGasParameters,
 };
-use move_binary_format::{access::ModuleAccess, errors::PartialVMError};
+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,
@@ -23,11 +23,11 @@ use move_core_types::{
     identifier::{IdentStr, Identifier},
     language_storage::{ModuleId, StructTag},
     metadata::Metadata,
-    resolver::{resource_size, ModuleResolver, ResourceResolver},
     value::MoveTypeLayout,
 };
 use move_prover_test_utils::{baseline_test::verify_or_update_baseline, extract_test_directives};
 use move_vm_test_utils::gas_schedule::GasStatus;
+use move_vm_types::resolver::{resource_size, ModuleResolver, ResourceResolver};
 use std::{
     cell::RefCell,
     collections::{BTreeMap, BTreeSet, VecDeque},
@@ -87,8 +87,8 @@ impl Harness {
         let mut gas = GasStatus::new_unmetered();
         let mut tick = 0;
         // Publish modules.
-        let mut proxy = HarnessProxy { harness: self };
-        let mut session = self.vm.new_session(test_account(), 0, &mut proxy);
+        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(&mut session, id, &mut gas, &mut done)?;
@@ -102,8 +102,8 @@ impl Harness {
                 actor.short_str_lossless()
             ));
             {
-                let mut proxy = HarnessProxy { harness: self };
-                let session = self.vm.new_session(addr, 0, &mut proxy);
+                let proxy = HarnessProxy { harness: self };
+                let session = self.vm.new_session(addr, 0, &proxy);
                 let result = session.new_actor(&actor, addr, &mut gas);
                 self.handle_result(&mut mailbox, result);
             };
@@ -133,8 +133,8 @@ impl Harness {
                 ))
             }
             // Handling
-            let mut proxy = HarnessProxy { harness: self };
-            let session = self.vm.new_session(actor, tick, &mut proxy);
+            let proxy = HarnessProxy { harness: self };
+            let session = self.vm.new_session(actor, tick, &proxy);
             tick += 1000_1000; // micros
             let result = session.handle_message(actor, message_hash, args, &mut gas);
             self.handle_result(&mut mailbox, result);
@@ -382,13 +382,11 @@ struct HarnessProxy<'a> {
 }
 
 impl<'a> ModuleResolver for HarnessProxy<'a> {
-    type Error = PartialVMError;
-
     fn get_module_metadata(&self, _module_id: &ModuleId) -> Vec<Metadata> {
         vec![]
     }
 
-    fn get_module(&self, id: &ModuleId) -> Result<Option<Bytes>, Self::Error> {
+    fn get_module(&self, id: &ModuleId) -> PartialVMResult<Option<Bytes>> {
         Ok(self
             .harness
             .module_cache
@@ -398,15 +396,13 @@ impl<'a> ModuleResolver for HarnessProxy<'a> {
 }
 
 impl<'a> ResourceResolver for HarnessProxy<'a> {
-    type Error = PartialVMError;
-
     fn get_resource_bytes_with_metadata_and_layout(
         &self,
         address: &AccountAddress,
         typ: &StructTag,
         _metadata: &[Metadata],
         _maybe_layout: Option<&MoveTypeLayout>,
-    ) -> Result<(Option<Bytes>, usize), Self::Error> {
+    ) -> PartialVMResult<(Option<Bytes>, usize)> {
         let res = self
             .harness
             .resource_store