Audit Move related TODOs in sui_core

crates/sui-core/src/authority.rs

@@ -2187,7 +2187,7 @@ impl AuthorityState {
         let object_ids = self
             .get_owner_objects_iterator(owner)?
             .filter(|o| match &o.type_ {
-                ObjectType::Struct(s) => Self::matches_type_fuzzy_generics(&type_, s),
+                ObjectType::Struct(s) => type_.matches_type_fuzzy_generics(s),
                 ObjectType::Package => false,
             })
             .map(|info| info.object_id);
@@ -2198,16 +2198,6 @@ impl AuthorityState {
         Ok(move_objects)
     }
 
-    // TODO: should be in impl MoveType
-    fn matches_type_fuzzy_generics(type_: &MoveObjectType, other_type: &MoveObjectType) -> bool {
-        type_.address() == other_type.address()
-                    && type_.module() == other_type.module()
-                    && type_.name() == other_type.name()
-                    // TODO: is_empty() looks like a bug here. I think the intention is to support "fuzzy matching" where `get_move_objects`
-                    // leaves type_params unspecified, but I don't actually see any call sites taking advantage of this
-                    && (type_.type_params().is_empty() || type_.type_params() == other_type.type_params())
-    }
-
     pub fn get_dynamic_fields(
         &self,
         owner: ObjectID,

crates/sui-core/src/authority/authority_store.rs

@@ -24,7 +24,8 @@ use sui_types::error::UserInputError;
 use sui_types::message_envelope::Message;
 use sui_types::object::Owner;
 use sui_types::storage::{
-    BackingPackageStore, ChildObjectResolver, DeleteKind, ObjectKey, ObjectStore,
+    get_module, get_module_by_id, BackingPackageStore, ChildObjectResolver, DeleteKind, ObjectKey,
+    ObjectStore,
 };
 use sui_types::sui_system_state::get_sui_system_state;
 use sui_types::{base_types::SequenceNumber, fp_bail, fp_ensure, storage::ParentSync};
@@ -1444,23 +1445,8 @@ impl ParentSync for AuthorityStore {
 impl ModuleResolver for AuthorityStore {
     type Error = SuiError;
 
-    // TODO: duplicated code with ModuleResolver for InMemoryStorage in memory_storage.rs.
     fn get_module(&self, module_id: &ModuleId) -> Result<Option<Vec<u8>>, Self::Error> {
-        // TODO: We should cache the deserialized modules to avoid
-        // fetching from the store / re-deserializing them every time.
-        // https://github.com/MystenLabs/sui/issues/809
-        Ok(self
-            .get_package(&ObjectID::from(*module_id.address()))?
-            .and_then(|package| {
-                // unwrap safe since get_package() ensures it's a package object.
-                package
-                    .data
-                    .try_as_package()
-                    .unwrap()
-                    .serialized_module_map()
-                    .get(module_id.name().as_str())
-                    .cloned()
-            }))
+        get_module(self, module_id)
     }
 }
 
@@ -1469,9 +1455,7 @@ impl GetModule for AuthorityStore {
     type Item = CompiledModule;
 
     fn get_module_by_id(&self, id: &ModuleId) -> anyhow::Result<Option<Self::Item>, Self::Error> {
-        Ok(self
-            .get_module(id)?
-            .map(|bytes| CompiledModule::deserialize(&bytes).unwrap()))
+        get_module_by_id(self, id)
     }
 }
 

crates/sui-types/src/base_types.rs

@@ -249,6 +249,15 @@ impl MoveObjectType {
             MoveObjectType::Other(o) => s == o,
         }
     }
+
+    pub fn matches_type_fuzzy_generics(&self, other_type: &Self) -> bool {
+        self.address() == other_type.address()
+            && self.module() == other_type.module()
+            && self.name() == other_type.name()
+            // MUSTFIX: is_empty() looks like a bug here. I think the intention is to support "fuzzy matching" where `get_move_objects`
+            // leaves type_params unspecified, but I don't actually see any call sites taking advantage of this
+            && (self.type_params().is_empty() || self.type_params() == other_type.type_params())
+    }
 }
 
 impl From<StructTag> for MoveObjectType {

crates/sui-types/src/in_memory_storage.rs

@@ -1,6 +1,7 @@
 // Copyright (c) Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
+use crate::storage::{get_module, get_module_by_id};
 use crate::{
     base_types::{ObjectID, ObjectRef, SequenceNumber},
     error::{SuiError, SuiResult},
@@ -58,19 +59,8 @@ impl ParentSync for InMemoryStorage {
 impl ModuleResolver for InMemoryStorage {
     type Error = SuiError;
 
-    // TODO: duplicated code with ModuleResolver for SuiDataStore in authority_store.rs.
     fn get_module(&self, module_id: &ModuleId) -> Result<Option<Vec<u8>>, Self::Error> {
-        Ok(self
-            .get_package(&ObjectID::from(*module_id.address()))?
-            .and_then(|package| {
-                package
-                    .data
-                    .try_as_package()
-                    .unwrap()
-                    .serialized_module_map()
-                    .get(module_id.name().as_str())
-                    .cloned()
-            }))
+        get_module(self, module_id)
     }
 }
 
@@ -99,9 +89,7 @@ impl GetModule for InMemoryStorage {
     type Item = CompiledModule;
 
     fn get_module_by_id(&self, id: &ModuleId) -> anyhow::Result<Option<Self::Item>, Self::Error> {
-        Ok(self
-            .get_module(id)?
-            .map(|bytes| CompiledModule::deserialize(&bytes).unwrap()))
+        get_module_by_id(self, id)
     }
 }
 

crates/sui-types/src/storage.rs

@@ -23,6 +23,8 @@ use crate::{
     event::Event,
     object::Object,
 };
+use move_binary_format::CompiledModule;
+use move_core_types::language_storage::ModuleId;
 use serde::{Deserialize, Serialize};
 use serde_with::serde_as;
 use std::collections::{BTreeMap, HashMap};
@@ -97,6 +99,29 @@ impl<S: BackingPackageStore> BackingPackageStore for &mut S {
     }
 }
 
+pub fn get_module<S: BackingPackageStore>(
+    store: S,
+    module_id: &ModuleId,
+) -> Result<Option<Vec<u8>>, SuiError> {
+    Ok(store
+        .get_package(&ObjectID::from(*module_id.address()))?
+        .and_then(|package| {
+            package.data.try_as_package().and_then(|package| {
+                package
+                    .serialized_module_map()
+                    .get(module_id.name().as_str())
+                    .cloned()
+            })
+        }))
+}
+
+pub fn get_module_by_id<S: BackingPackageStore>(
+    store: S,
+    id: &ModuleId,
+) -> anyhow::Result<Option<CompiledModule>, SuiError> {
+    Ok(get_module(store, id)?.map(|bytes| CompiledModule::deserialize(&bytes).unwrap()))
+}
+
 pub trait ParentSync {
     fn get_latest_parent_entry_ref(&self, object_id: ObjectID) -> SuiResult<Option<ObjectRef>>;
 }