[Move adapter] Fixed framework modules being accidentally marked for …

…publishing (MystenLabs#1085)

* [Move adapter] Fixed framework modules being accidentally marked for publishing

* Fixed a clippy warning

* Addressed review comment

sui_programmability/adapter/src/adapter.rs

@@ -4,7 +4,9 @@
 use anyhow::Result;
 
 use crate::bytecode_rewriter::ModuleHandleRewriter;
-use move_binary_format::{errors::PartialVMResult, file_format::CompiledModule, normalized::Type};
+use move_binary_format::{
+    access::ModuleAccess, errors::PartialVMResult, file_format::CompiledModule, normalized::Type,
+};
 use sui_framework::EventType;
 use sui_types::{
     base_types::*,
@@ -527,8 +529,10 @@ pub fn generate_package_id(
         let old_module_id = module.self_id();
         let old_address = *old_module_id.address();
         if old_address != AccountAddress::ZERO {
+            let handle = module.module_handle_at(module.self_module_handle_idx);
+            let name = module.identifier_at(handle.name);
             return Err(SuiError::ModulePublishFailure {
-                error: "Publishing modules with non-zero address is not allowed".to_string(),
+                error: format!("Publishing module {name} with non-zero address is not allowed"),
             });
         }
         let new_module_id = ModuleId::new(

sui_programmability/adapter/src/unit_tests/adapter_tests.rs

@@ -848,7 +848,10 @@ fn test_publish_module_non_zero_address() {
     assert_eq!(err.0, gas::MIN_MOVE);
     let err_str = err.1.to_string();
     println!("{:?}", err_str);
-    assert!(err_str.contains("Publishing modules with non-zero address is not allowed"));
+    assert!(
+        err_str.contains("Publishing module")
+            && err_str.contains("with non-zero address is not allowed")
+    );
 }
 
 #[test]

sui_programmability/framework/src/lib.rs

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use move_binary_format::CompiledModule;
-use move_core_types::{account_address::AccountAddress, ident_str};
+use move_core_types::{account_address::AccountAddress, ident_str, language_storage::ModuleId};
 use move_package::BuildConfig;
 use move_unit_test::UnitTestingConfig;
 use num_enum::TryFromPrimitive;
@@ -116,21 +116,25 @@ pub fn build_move_package(
                     });
                 }
             }
-            // Collect all module names from the current package to be published.
+            // Collect all module IDs from the current package to be
+            // published (module names are not sufficient as we may
+            // have modules with the same names in user code and in
+            // Sui framework which would result in the latter being
+            // pulled into a set of modules to be published).
             // For each transitive dependent module, if they are not to be published,
             // they must have a non-zero address (meaning they are already published on-chain).
             // TODO: Shall we also check if they are really on-chain in the future?
-            let self_modules: HashSet<String> = compiled_modules
+            let self_modules: HashSet<ModuleId> = compiled_modules
                 .iter_modules()
                 .iter()
-                .map(|m| m.self_id().name().to_string())
+                .map(|m| m.self_id())
                 .collect();
             if let Some(m) = package
                 .transitive_compiled_modules()
                 .iter_modules()
                 .iter()
                 .find(|m| {
-                    !self_modules.contains(m.self_id().name().as_str())
+                    !self_modules.contains(&m.self_id())
                         && m.self_id().address() == &AccountAddress::ZERO
                 })
             {
@@ -141,7 +145,7 @@ pub fn build_move_package(
                 .compute_dependency_graph()
                 .compute_topological_order()
                 .unwrap()
-                .filter(|m| self_modules.contains(m.self_id().name().as_str()))
+                .filter(|m| self_modules.contains(&m.self_id()))
                 .cloned()
                 .collect())
         }