[Package management] add implicit system package dependencies (dvx-197)

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

@@ -34,15 +34,21 @@ use move_package::{
     },
     package_hooks::{PackageHooks, PackageIdentifier},
     resolution::{dependency_graph::DependencyGraph, resolution_graph::ResolvedGraph},
-    source_package::parsed_manifest::{Dependencies, PackageName},
-    BuildConfig as MoveBuildConfig, LintFlag,
+    source_package::parsed_manifest::{
+        Dependencies, Dependency, DependencyKind, GitInfo, InternalDependency, PackageName,
+    },
+    BuildConfig as MoveBuildConfig,
 };
 use move_package::{
     source_package::parsed_manifest::OnChainInfo, source_package::parsed_manifest::SourceManifest,
 };
 use move_symbol_pool::Symbol;
 use serde_reflection::Registry;
-use sui_package_management::{resolve_published_id, PublishedAtError};
+use sui_package_management::{
+    resolve_published_id,
+    system_package_versions::{SystemPackagesVersion, SYSTEM_GIT_REPO},
+    PublishedAtError,
+};
 use sui_protocol_config::{Chain, ProtocolConfig, ProtocolVersion};
 use sui_types::{
     base_types::ObjectID,
@@ -110,18 +116,17 @@ pub struct BuildConfig {
 impl BuildConfig {
     pub fn new_for_testing() -> Self {
         move_package::package_hooks::register_package_hooks(Box::new(SuiPackageHooks));
-
-        // Note: in the future, consider changing this to dependencies on the local system
-        // packages:
-        let implicit_dependencies = Dependencies::new();
         let install_dir = tempfile::tempdir().unwrap().into_path();
+
         let config = MoveBuildConfig {
             default_flavor: Some(move_compiler::editions::Flavor::Sui),
-            implicit_dependencies,
+
             lock_file: Some(install_dir.join("Move.lock")),
             install_dir: Some(install_dir),
-            lint_flag: LintFlag::LEVEL_NONE,
             silence_warnings: true,
+            lint_flag: move_package::LintFlag::LEVEL_NONE,
+            // TODO[DVX-793]: in the future, we may want to provide local implicit dependencies to tests
+            implicit_dependencies: Dependencies::new(),
             ..MoveBuildConfig::default()
         };
         BuildConfig {
@@ -299,7 +304,7 @@ pub fn build_from_resolution_graph(
     })
 }
 
-/// Returns the deps from `resolution_graph` that have no source code
+/// Returns the bytecode deps from `resolution_graph` that have no source code
 fn collect_bytecode_deps(
     resolution_graph: &ResolvedGraph,
 ) -> SuiResult<Vec<(Symbol, CompiledModule)>> {
@@ -709,6 +714,30 @@ impl CompiledPackage {
     }
 }
 
+/// Create a set of [Dependencies] from a [SystemPackagesVersion]; the dependencies are override git
+/// dependencies to the specific revision given by the [SystemPackagesVersion]
+pub fn implicit_deps(packages: &SystemPackagesVersion) -> Dependencies {
+    packages
+        .packages
+        .iter()
+        .map(|package| {
+            (
+                package.package_name.clone().into(),
+                Dependency::Internal(InternalDependency {
+                    kind: DependencyKind::Git(GitInfo {
+                        git_url: SYSTEM_GIT_REPO.into(),
+                        git_rev: packages.git_revision.clone().into(),
+                        subdir: package.repo_path.clone().into(),
+                    }),
+                    subst: None,
+                    digest: None,
+                    dep_override: true,
+                }),
+            )
+        })
+        .collect()
+}
+
 impl GetModule for CompiledPackage {
     type Error = anyhow::Error;
     // TODO: return ref here for better efficiency? Borrow checker + all_modules_map() make it hard to do this

crates/sui-move/Cargo.toml

@@ -34,6 +34,7 @@ sui-move-natives = { path = "../../sui-execution/latest/sui-move-natives", packa
 sui-move-build.workspace = true
 sui-protocol-config.workspace = true
 sui-types.workspace = true
+sui-package-management.workspace = true
 better_any = "0.1.1"
 
 [target.'cfg(not(target_env = "msvc"))'.dependencies]

crates/sui-move/src/build.rs

@@ -7,7 +7,10 @@ use move_cli::base;
 use move_package::BuildConfig as MoveBuildConfig;
 use serde_json::json;
 use std::{fs, path::Path};
-use sui_move_build::{check_invalid_dependencies, check_unpublished_dependencies, BuildConfig};
+use sui_move_build::{
+    check_invalid_dependencies, check_unpublished_dependencies, implicit_deps, BuildConfig,
+};
+use sui_package_management::system_package_versions::latest_system_packages;
 
 const LAYOUTS_DIR: &str = "layouts";
 const STRUCT_LAYOUTS_FILENAME: &str = "struct_layouts.yaml";
@@ -61,12 +64,13 @@ impl Build {
 
     pub fn execute_internal(
         rerooted_path: &Path,
-        config: MoveBuildConfig,
+        mut config: MoveBuildConfig,
         with_unpublished_deps: bool,
         dump_bytecode_as_base64: bool,
         generate_struct_layouts: bool,
         chain_id: Option<String>,
     ) -> anyhow::Result<()> {
+        config.implicit_dependencies = implicit_deps(latest_system_packages());
         let mut pkg = BuildConfig {
             config,
             run_bytecode_verifier: true,

crates/sui-move/src/new.rs

@@ -6,11 +6,6 @@ use move_cli::base::new;
 use move_package::source_package::layout::SourcePackageLayout;
 use std::{fs::create_dir_all, io::Write, path::Path};
 
-const SUI_PKG_NAME: &str = "Sui";
-
-// Use testnet by default. Probably want to add options to make this configurable later
-const SUI_PKG_PATH: &str = "{ git = \"https://github.com/MystenLabs/sui.git\", subdir = \"crates/sui-framework/packages/sui-framework\", rev = \"framework/testnet\", override = true }";
-
 #[derive(Parser)]
 #[group(id = "sui-move-new")]
 pub struct New {
@@ -24,7 +19,7 @@ impl New {
         let provided_name = &self.new.name.to_string();
 
         self.new
-            .execute(path, [(SUI_PKG_NAME, SUI_PKG_PATH)], [(name, "0x0")], "")?;
+            .execute(path, [] as [(&str, &str); 0], [(name, "0x0")], "")?;
         let p = path.unwrap_or_else(|| Path::new(&provided_name));
         let mut w = std::fs::File::create(
             p.join(SourcePackageLayout::Sources.path())

crates/sui-move/src/unit_test.rs

@@ -11,9 +11,10 @@ use move_unit_test::{extensions::set_extension_hook, UnitTestingConfig};
 use move_vm_runtime::native_extensions::NativeContextExtensions;
 use once_cell::sync::Lazy;
 use std::{cell::RefCell, collections::BTreeMap, path::Path, sync::Arc};
-use sui_move_build::decorate_warnings;
+use sui_move_build::{decorate_warnings, implicit_deps};
 use sui_move_natives::test_scenario::InMemoryTestStore;
 use sui_move_natives::{object_runtime::ObjectRuntime, NativesCostTable};
+use sui_package_management::system_package_versions::latest_system_packages;
 use sui_protocol_config::ProtocolConfig;
 use sui_types::{
     gas_model::tables::initial_cost_schedule_for_unit_tests, in_memory_storage::InMemoryStorage,
@@ -71,7 +72,7 @@ static SET_EXTENSION_HOOK: Lazy<()> =
 /// successfully started running the test, and the inner result indicatests whether all tests pass.
 pub fn run_move_unit_tests(
     path: &Path,
-    build_config: BuildConfig,
+    mut build_config: BuildConfig,
     config: Option<UnitTestingConfig>,
     compute_coverage: bool,
     save_disassembly: bool,
@@ -81,6 +82,7 @@ pub fn run_move_unit_tests(
 
     let config = config
         .unwrap_or_else(|| UnitTestingConfig::default_with_bound(Some(MAX_UNIT_TEST_INSTRUCTIONS)));
+    build_config.implicit_dependencies = implicit_deps(latest_system_packages());
 
     let result = move_cli::base::test::run_move_unit_tests(
         path,

crates/sui-package-management/src/system_package_versions.rs

@@ -16,6 +16,8 @@ static VERSION_TABLE: LazyLock<BTreeMap<ProtocolVersion, SystemPackagesVersion>>
         )))
     });
 
+pub const SYSTEM_GIT_REPO: &str = "https://github.com/MystenLabs/sui.git";
+
 #[derive(Debug)]
 pub struct SystemPackagesVersion {
     pub git_revision: String,

crates/sui-source-validation-service/Cargo.toml

@@ -27,6 +27,7 @@ url = "2.3.1"
 
 sui-move.workspace = true
 sui-move-build.workspace = true
+sui-package-management.workspace = true
 sui-sdk.workspace = true
 sui-source-validation.workspace = true
 

crates/sui-source-validation-service/src/lib.rs

@@ -9,6 +9,7 @@ use std::path::PathBuf;
 use std::sync::{Arc, RwLock};
 use std::time::Duration;
 use std::{ffi::OsString, fs, path::Path, process::Command};
+use sui_package_management::system_package_versions::latest_system_packages;
 use tokio::sync::oneshot::Sender;
 
 use anyhow::{anyhow, bail};
@@ -30,7 +31,7 @@ use move_core_types::account_address::AccountAddress;
 use move_package::{BuildConfig as MoveBuildConfig, LintFlag};
 use move_symbol_pool::Symbol;
 use sui_move::manage_package::resolve_lock_file_path;
-use sui_move_build::{BuildConfig, SuiPackageHooks};
+use sui_move_build::{implicit_deps, BuildConfig, SuiPackageHooks};
 use sui_sdk::rpc_types::SuiTransactionBlockEffects;
 use sui_sdk::types::base_types::ObjectID;
 use sui_sdk::SuiClientBuilder;
@@ -163,6 +164,7 @@ pub async fn verify_package(
         resolve_lock_file_path(MoveBuildConfig::default(), Some(package_path.as_ref()))?;
     config.lint_flag = LintFlag::LEVEL_NONE;
     config.silence_warnings = true;
+    config.implicit_dependencies = implicit_deps(latest_system_packages());
     let build_config = BuildConfig {
         config,
         run_bytecode_verifier: false, /* no need to run verifier if code is on-chain */

crates/sui-source-validation/src/lib.rs

@@ -234,6 +234,12 @@ impl ValidationMode {
                     }
                 })?;
 
+            // only keep modules that are actually used
+            let deps_compiled_units: Vec<_> = deps_compiled_units
+                .into_iter()
+                .filter(|pkg| sui_package.dependency_ids.published.contains_key(&pkg.0))
+                .collect();
+
             for (package, local_unit) in deps_compiled_units {
                 let m = &local_unit.unit;
                 let module = m.name;

crates/sui/src/client_commands.rs

@@ -31,7 +31,7 @@ use reqwest::StatusCode;
 use move_binary_format::CompiledModule;
 use move_bytecode_verifier_meter::Scope;
 use move_core_types::{account_address::AccountAddress, language_storage::TypeTag};
-use move_package::BuildConfig as MoveBuildConfig;
+use move_package::{source_package::parsed_manifest::Dependencies, BuildConfig as MoveBuildConfig};
 use prometheus::Registry;
 use serde::Serialize;
 use serde_json::{json, Value};
@@ -52,9 +52,12 @@ use sui_json_rpc_types::{
 use sui_keys::keystore::AccountKeystore;
 use sui_move_build::{
     build_from_resolution_graph, check_invalid_dependencies, check_unpublished_dependencies,
-    gather_published_ids, BuildConfig, CompiledPackage,
+    gather_published_ids, implicit_deps, BuildConfig, CompiledPackage,
+};
+use sui_package_management::{
+    system_package_versions::{latest_system_packages, system_packages_for_protocol},
+    LockCommand, PublishedAtError,
 };
-use sui_package_management::{LockCommand, PublishedAtError};
 use sui_replay::ReplayToolCommand;
 use sui_sdk::{
     apis::ReadApi,
@@ -1677,7 +1680,7 @@ impl SuiClientCommands {
             ),
             SuiClientCommands::VerifySource {
                 package_path,
-                build_config,
+                mut build_config,
                 verify_deps,
                 skip_source,
                 address_override,
@@ -1694,6 +1697,7 @@ impl SuiClientCommands {
                     (true, true, Some(at)) => ValidationMode::root_and_deps_at(*at),
                 };
 
+                build_config.implicit_dependencies = implicit_deps(latest_system_packages());
                 let build_config = resolve_lock_file_path(build_config, Some(&package_path))?;
                 let chain_id = context
                     .get_client()
@@ -1763,10 +1767,11 @@ fn check_dep_verification_flags(
 }
 
 fn compile_package_simple(
-    build_config: MoveBuildConfig,
+    mut build_config: MoveBuildConfig,
     package_path: &Path,
     chain_id: Option<String>,
 ) -> Result<CompiledPackage, anyhow::Error> {
+    build_config.implicit_dependencies = implicit_deps(latest_system_packages());
     let config = BuildConfig {
         config: resolve_lock_file_path(build_config, Some(package_path))?,
         run_bytecode_verifier: false,
@@ -1855,11 +1860,15 @@ pub(crate) async fn upgrade_package(
 
 pub(crate) async fn compile_package(
     read_api: &ReadApi,
-    build_config: MoveBuildConfig,
+    mut build_config: MoveBuildConfig,
     package_path: &Path,
     with_unpublished_dependencies: bool,
     skip_dependency_verification: bool,
 ) -> Result<CompiledPackage, anyhow::Error> {
+    let protocol_config = read_api.get_protocol_config(None).await?;
+
+    build_config.implicit_dependencies =
+        implicit_deps_for_protocol_version(protocol_config.protocol_version)?;
     let config = resolve_lock_file_path(build_config, Some(package_path))?;
     let run_bytecode_verifier = true;
     let print_diags_to_stderr = true;
@@ -1989,6 +1998,23 @@ pub(crate) async fn compile_package(
     Ok(compiled_package)
 }
 
+/// Return the correct implicit dependencies for the [version], producing a warning or error if the
+/// protocol version is unknown or old
+fn implicit_deps_for_protocol_version(version: ProtocolVersion) -> anyhow::Result<Dependencies> {
+    if version > ProtocolVersion::MAX + 2 {
+        eprintln!(
+            "[{}]: The network is using protocol version {:?}, but this binary only recognizes protocol version {:?}; \
+            the system packages used for compilation (e.g. MoveStdlib) may be out of date. If you have errors related to \
+            system packages, you may need to update your CLI.",
+            "warning".bold().yellow(),
+            ProtocolVersion::MAX,
+            version
+        )
+    }
+
+    Ok(implicit_deps(system_packages_for_protocol(version)?.0))
+}
+
 impl Display for SuiClientCommandResult {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         let mut writer = String::new();

crates/sui/tests/data/module_dependency_invalid/Move.toml

@@ -4,5 +4,8 @@ version = "0.0.1"
 published-at = "mystery"
 edition = "2024.beta"
 
+[dependencies]
+MoveStdlib = { local = "../../../../sui-framework/packages/move-stdlib" }
+
 [addresses]
 invalid = "0x0"

crates/sui/tests/data/module_dependency_nonexistent/Move.toml

@@ -4,5 +4,8 @@ version = "0.0.1"
 published-at = "0xabc123"
 edition = "2024.beta"
 
+[dependencies]
+MoveStdlib = { local = "../../../../sui-framework/packages/move-stdlib" }
+
 [addresses]
 nonexistent = "0x0"

crates/sui/tests/data/module_dependency_unpublished/Move.toml

@@ -5,5 +5,8 @@ edition = "2024.beta"
 # No published-at address for this package.
 # published-at = "0x0"
 
+[dependencies]
+MoveStdlib = { local = "../../../../sui-framework/packages/move-stdlib" }
+
 [addresses]
 invalid = "0x0"

crates/sui/tests/data/module_dependency_unpublished_non_zero_address/Move.toml

@@ -5,6 +5,9 @@ edition = "2024.beta"
 # No published-at address for this package.
 # published-at = "0x0"
 
+[dependencies]
+MoveStdlib = { local = "../../../../sui-framework/packages/move-stdlib" }
+
 [addresses]
 # Address not set to 0x0. Error if --with-unpublished-dependencies is set.
 non_zero = "0xbad"

crates/sui/tests/shell_tests/implicits/build.sh

@@ -0,0 +1,5 @@
+# Copyright (c) Mysten Labs, Inc.
+# SPDX-License-Identifier: Apache-2.0
+
+# tests that building a package that implicitly depends on `Bridge` can build
+sui move build -p example 2> /dev/null

crates/sui/tests/shell_tests/implicits/build_dev.sh

@@ -0,0 +1,5 @@
+# Copyright (c) Mysten Labs, Inc.
+# SPDX-License-Identifier: Apache-2.0
+
+# tests that building a package that implicitly depends on `Bridge` works in dev mode
+sui move build --dev -p example 2> /dev/null

crates/sui/tests/shell_tests/implicits/example/Move.toml

@@ -0,0 +1,6 @@
+[package]
+name = "example"
+edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move
+
+[addresses]
+example = "0x0"