[move] Implemented detection of Move version mismatch (#3967)

* [move] Implemented detection of Move version mismatch

* Added additional comment

* Addressed review comments

* Addressed more review comments

crates/sui-framework-build/Cargo.toml

@@ -9,10 +9,13 @@ publish = false
 
 [dependencies]
 
+once_cell = "1.11.0"
+
 sui-types = { path = "../sui-types" }
 sui-verifier = { path = "../../crates/sui-verifier" }
 
 move-binary-format = { git = "https://github.com/move-language/move", rev = "79071528524f08b12e9abb84c1094d8e976aa17a" }
+move-bytecode-utils = { git = "https://github.com/move-language/move", rev = "79071528524f08b12e9abb84c1094d8e976aa17a" }
 move-bytecode-verifier = { git = "https://github.com/move-language/move", rev = "79071528524f08b12e9abb84c1094d8e976aa17a" }
 move-compiler = { git = "https://github.com/move-language/move", rev = "79071528524f08b12e9abb84c1094d8e976aa17a" }
 move-core-types = { git = "https://github.com/move-language/move", rev = "79071528524f08b12e9abb84c1094d8e976aa17a", features = ["address20"] }

crates/sui-framework-build/src/lib.rs

@@ -3,27 +3,31 @@
 
 use move_binary_format::CompiledModule;
 use move_compiler::compiled_unit::{CompiledUnit, NamedCompiledModule};
-use move_core_types::{account_address::AccountAddress, ident_str, language_storage::ModuleId};
-use move_package::BuildConfig;
+use move_core_types::{account_address::AccountAddress, language_storage::ModuleId};
+use move_package::{compilation::compiled_package::CompiledPackage, BuildConfig};
 use std::{collections::HashSet, path::Path};
 use sui_types::error::{SuiError, SuiResult};
 use sui_verifier::verifier as sui_bytecode_verifier;
 
 const SUI_PACKAGE_NAME: &str = "Sui";
 const MOVE_STDLIB_PACKAGE_NAME: &str = "MoveStdlib";
 
-pub fn build_move_stdlib_modules(lib_dir: &Path) -> SuiResult<Vec<CompiledModule>> {
-    let denylist = vec![
-        ident_str!("capability").to_owned(),
-        ident_str!("event").to_owned(),
-        ident_str!("guid").to_owned(),
+pub fn move_stdlib_module_denylist() -> Vec<String> {
+    vec![
+        "capability".to_string(),
+        "event".to_string(),
+        "guid".to_string(),
         #[cfg(not(test))]
-        ident_str!("debug").to_owned(),
-    ];
+        "debug".to_string(),
+    ]
+}
+
+pub fn build_move_stdlib_modules(lib_dir: &Path) -> SuiResult<Vec<CompiledModule>> {
     let build_config = BuildConfig::default();
-    let modules: Vec<CompiledModule> = build_move_package(lib_dir, build_config)?
+    let pkg = build_move_package_with_deps(lib_dir, build_config)?;
+    let modules: Vec<CompiledModule> = filter_package_modules(&pkg)?
         .into_iter()
-        .filter(|m| !denylist.contains(&m.self_id().name().to_owned()))
+        .filter(|m| !move_stdlib_module_denylist().contains(&m.self_id().name().to_string()))
         .collect();
     verify_modules(&modules)?;
     Ok(modules)
@@ -41,12 +45,13 @@ pub fn verify_modules(modules: &[CompiledModule]) -> SuiResult {
     Ok(())
     // TODO(https://github.com/MystenLabs/sui/issues/69): Run Move linker
 }
-/// Given a `path` and a `build_config`, build the package in that path.
+
+/// Given a `path` and a `build_config`, build the package in that path, including its dependencies.
 /// If we are building the Sui framework, we skip the check that the addresses should be 0
-pub fn build_move_package(
+pub fn build_move_package_with_deps(
     path: &Path,
     build_config: BuildConfig,
-) -> SuiResult<Vec<CompiledModule>> {
+) -> SuiResult<CompiledPackage> {
     match build_config.compile_package_no_exit(path, &mut Vec::new()) {
         Err(error) => Err(SuiError::ModuleBuildFailure {
             error: error.to_string(),
@@ -70,43 +75,49 @@ pub fn build_move_package(
                     });
                 }
             }
-            // 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<ModuleId> = compiled_modules
-                .iter_modules()
-                .iter()
-                .map(|m| m.self_id())
-                .collect();
-            if let Some(m) =
-                package
-                    .deps_compiled_units
-                    .iter()
-                    .find_map(|(_, unit)| match &unit.unit {
-                        CompiledUnit::Module(NamedCompiledModule { module: m, .. })
-                            if !self_modules.contains(&m.self_id())
-                                && m.self_id().address() == &AccountAddress::ZERO =>
-                        {
-                            Some(m)
-                        }
-                        _ => None,
-                    })
+            Ok(package)
+        }
+    }
+}
+
+/// Given a package bundled with its dependencies, filter out modules that only belong to this
+/// package.
+pub fn filter_package_modules(package: &CompiledPackage) -> SuiResult<Vec<CompiledModule>> {
+    let compiled_modules = package.root_modules_map();
+    // 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<ModuleId> = compiled_modules
+        .iter_modules()
+        .iter()
+        .map(|m| m.self_id())
+        .collect();
+    if let Some(m) = package
+        .deps_compiled_units
+        .iter()
+        .find_map(|(_, unit)| match &unit.unit {
+            CompiledUnit::Module(NamedCompiledModule { module: m, .. })
+                if !self_modules.contains(&m.self_id())
+                    && m.self_id().address() == &AccountAddress::ZERO =>
             {
-                return Err(SuiError::ModulePublishFailure { error: format!("Dependent modules must have been published on-chain with non-0 addresses, unlike module {:?}", m.self_id()) });
+                Some(m)
             }
-            Ok(package
-                .all_modules_map()
-                .compute_dependency_graph()
-                .compute_topological_order()
-                .unwrap()
-                .filter(|m| self_modules.contains(&m.self_id()))
-                .cloned()
-                .collect())
-        }
+            _ => None,
+        })
+    {
+        return Err(SuiError::ModulePublishFailure { error: format!("Dependent modules must have been published on-chain with non-0 addresses, unlike module {:?}", m.self_id()) });
     }
+    Ok(package
+        .all_modules_map()
+        .compute_dependency_graph()
+        .compute_topological_order()
+        .unwrap()
+        .filter(|m| self_modules.contains(&m.self_id()))
+        .cloned()
+        .collect())
 }

crates/sui-framework/Cargo.toml

@@ -18,9 +18,13 @@ sha3 = "0.10.1"
 
 sui-types = { path = "../sui-types" }
 sui-framework-build = { path = "../sui-framework-build" }
+sui-verifier = { path = "../../crates/sui-verifier" }
 
 move-binary-format = { git = "https://github.com/move-language/move", rev = "79071528524f08b12e9abb84c1094d8e976aa17a" }
+move-bytecode-utils = { git = "https://github.com/move-language/move", rev = "79071528524f08b12e9abb84c1094d8e976aa17a" }
+move-bytecode-verifier = { git = "https://github.com/move-language/move", rev = "79071528524f08b12e9abb84c1094d8e976aa17a" }
 move-cli = { git = "https://github.com/move-language/move", rev = "79071528524f08b12e9abb84c1094d8e976aa17a" }
+move-compiler = { git = "https://github.com/move-language/move", rev = "79071528524f08b12e9abb84c1094d8e976aa17a" }
 move-core-types = { git = "https://github.com/move-language/move", rev = "79071528524f08b12e9abb84c1094d8e976aa17a", features = ["address20"] }
 move-package = { git = "https://github.com/move-language/move", rev = "79071528524f08b12e9abb84c1094d8e976aa17a" }
 move-stdlib = { git = "https://github.com/move-language/move", rev = "79071528524f08b12e9abb84c1094d8e976aa17a" }

crates/sui-framework/build.rs

@@ -51,9 +51,12 @@ fn build_framework_and_stdlib(
     sui_framework_path: &Path,
     move_stdlib_path: &Path,
 ) -> (Vec<CompiledModule>, Vec<CompiledModule>) {
-    let sui_framework =
-        sui_framework_build::build_move_package(sui_framework_path, BuildConfig::default())
-            .unwrap();
+    let pkg = sui_framework_build::build_move_package_with_deps(
+        sui_framework_path,
+        BuildConfig::default(),
+    )
+    .unwrap();
+    let sui_framework = sui_framework_build::filter_package_modules(&pkg).unwrap();
     let move_stdlib = sui_framework_build::build_move_stdlib_modules(move_stdlib_path).unwrap();
     (sui_framework, move_stdlib)
 }

crates/sui-framework/src/lib.rs

@@ -2,19 +2,26 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use move_binary_format::CompiledModule;
+use move_bytecode_utils::Modules;
 use move_cli::base::test::UnitTestResult;
-use move_package::BuildConfig;
+use move_package::{compilation::compiled_package::CompiledPackage, BuildConfig};
 use move_unit_test::UnitTestingConfig;
 use num_enum::TryFromPrimitive;
 use once_cell::sync::Lazy;
 use std::path::Path;
-use sui_types::error::{SuiError, SuiResult};
+use sui_types::{
+    error::{SuiError, SuiResult},
+    MOVE_STDLIB_ADDRESS, SUI_FRAMEWORK_ADDRESS,
+};
 
 pub mod cost_calib;
 pub mod natives;
 
 pub use sui_framework_build::build_move_stdlib_modules as get_move_stdlib_modules;
-pub use sui_framework_build::{build_move_package, verify_modules};
+pub use sui_framework_build::verify_modules;
+use sui_framework_build::{
+    build_move_package_with_deps, filter_package_modules, move_stdlib_module_denylist,
+};
 use sui_types::sui_serde::{Base64, Encoding};
 
 // Move unit tests will halt after executing this many steps. This is a protection to avoid divergence
@@ -114,8 +121,6 @@ pub fn run_move_unit_tests(
     config: Option<UnitTestingConfig>,
     compute_coverage: bool,
 ) -> anyhow::Result<UnitTestResult> {
-    use sui_types::{MOVE_STDLIB_ADDRESS, SUI_FRAMEWORK_ADDRESS};
-
     let config = config
         .unwrap_or_else(|| UnitTestingConfig::default_with_bound(Some(MAX_UNIT_TEST_INSTRUCTIONS)));
 
@@ -132,6 +137,62 @@ pub fn run_move_unit_tests(
     )
 }
 
+pub fn build_move_package(
+    path: &Path,
+    build_config: BuildConfig,
+) -> SuiResult<Vec<CompiledModule>> {
+    let pkg = build_move_package_with_deps(path, build_config)?;
+    verify_framework_version(&pkg)?;
+    filter_package_modules(&pkg)
+}
+
+/// Version of the framework code that the binary used for compilation expects should be the same as
+/// version of the framework code bundled as compiled package's dependency and this function
+/// verifies this.
+fn verify_framework_version(pkg: &CompiledPackage) -> SuiResult<()> {
+    // We stash compiled modules in the Modules map which is sorted so that we can compare sets of
+    // compiled modules directly.
+
+    let dep_framework_modules = pkg.all_modules_map().iter_modules_owned();
+    let dep_framework: Vec<&CompiledModule> = dep_framework_modules
+        .iter()
+        .filter(|m| *m.self_id().address() == SUI_FRAMEWORK_ADDRESS)
+        .collect();
+
+    let framework_modules = Modules::new(get_sui_framework().iter()).iter_modules_owned();
+    let framework: Vec<&CompiledModule> = framework_modules.iter().collect();
+
+    if dep_framework != framework {
+        return Err(SuiError::ModuleVerificationFailure {
+            error: "Sui framework version mismatch detected.\
+                    Make sure that the sui command line tool and the Sui framework code\
+                    used as a dependency correspond to the same git commit"
+                .to_string(),
+        });
+    }
+
+    let dep_stdlib_modules = pkg.all_modules_map().iter_modules_owned();
+    let dep_stdlib: Vec<&CompiledModule> = dep_stdlib_modules
+        .iter()
+        .filter(|m| *m.self_id().address() == MOVE_STDLIB_ADDRESS)
+        .filter(|m| !move_stdlib_module_denylist().contains(&m.self_id().name().to_string()))
+        .collect();
+
+    let stdlib_modules = Modules::new(get_move_stdlib().iter()).iter_modules_owned();
+    let stdlib: Vec<&CompiledModule> = stdlib_modules.iter().collect();
+
+    if dep_stdlib != stdlib {
+        return Err(SuiError::ModuleVerificationFailure {
+            error: "Move stdlib version mismatch detected.\
+                    Make sure that the sui command line tool and the Move standard library code\
+                    used as a dependency correspond to the same git commit"
+                .to_string(),
+        });
+    }
+
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;