From e0f01414342265a1c944b53649ce92a7a3ba2f25 Mon Sep 17 00:00:00 2001 From: Michael George Date: Fri, 28 Feb 2025 10:34:54 -0500 Subject: [PATCH 1/5] Added sui implicit dependencies everywhere a sui `BuildConfig` is created --- Cargo.lock | 2 + crates/sui-move-build/src/lib.rs | 47 +++++++++++++++---- crates/sui-move/Cargo.toml | 1 + crates/sui-move/src/build.rs | 8 +++- .../src/system_package_versions.rs | 2 + .../sui-source-validation-service/Cargo.toml | 1 + .../sui-source-validation-service/src/lib.rs | 4 +- crates/sui/src/client_commands.rs | 36 +++++++++++--- 8 files changed, 83 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 327e304a530b2..38f173613a596 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14232,6 +14232,7 @@ dependencies = [ "sui-macros", "sui-move-build", "sui-move-natives-latest", + "sui-package-management", "sui-protocol-config", "sui-types", "telemetry-subscribers", @@ -15195,6 +15196,7 @@ dependencies = [ "sui-json-rpc-types", "sui-move", "sui-move-build", + "sui-package-management", "sui-sdk", "sui-source-validation", "telemetry-subscribers", diff --git a/crates/sui-move-build/src/lib.rs b/crates/sui-move-build/src/lib.rs index f41c268194bd9..85e1ee03f3864 100644 --- a/crates/sui-move-build/src/lib.rs +++ b/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: in the future this should probably be changed to a set of local deps: + implicit_dependencies: Dependencies::new(), ..MoveBuildConfig::default() }; BuildConfig { @@ -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 diff --git a/crates/sui-move/Cargo.toml b/crates/sui-move/Cargo.toml index b6bfeb84b9472..13584cdae882a 100644 --- a/crates/sui-move/Cargo.toml +++ b/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] diff --git a/crates/sui-move/src/build.rs b/crates/sui-move/src/build.rs index 9c5ded8e4d077..07d634b22f0ef 100644 --- a/crates/sui-move/src/build.rs +++ b/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, ) -> anyhow::Result<()> { + config.implicit_dependencies = implicit_deps(latest_system_packages()); let mut pkg = BuildConfig { config, run_bytecode_verifier: true, diff --git a/crates/sui-package-management/src/system_package_versions.rs b/crates/sui-package-management/src/system_package_versions.rs index 55a6def42c802..339006dfebd5c 100644 --- a/crates/sui-package-management/src/system_package_versions.rs +++ b/crates/sui-package-management/src/system_package_versions.rs @@ -16,6 +16,8 @@ static VERSION_TABLE: LazyLock> ))) }); +pub const SYSTEM_GIT_REPO: &str = "https://github.com/MystenLabs/sui.git"; + #[derive(Debug)] pub struct SystemPackagesVersion { pub git_revision: String, diff --git a/crates/sui-source-validation-service/Cargo.toml b/crates/sui-source-validation-service/Cargo.toml index 36f1bf325d4f1..4a27d4b0a1f49 100644 --- a/crates/sui-source-validation-service/Cargo.toml +++ b/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 diff --git a/crates/sui-source-validation-service/src/lib.rs b/crates/sui-source-validation-service/src/lib.rs index ff95996921ea5..547832b257787 100644 --- a/crates/sui-source-validation-service/src/lib.rs +++ b/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 */ diff --git a/crates/sui/src/client_commands.rs b/crates/sui/src/client_commands.rs index 4a34db4fc4b42..9df2282efa048 100644 --- a/crates/sui/src/client_commands.rs +++ b/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, ) -> Result { + 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 { + let protocol_config = read_api.get_protocol_config(None).await?; + let protocol_version = protocol_config.protocol_version; + + build_config.implicit_dependencies = implicit_deps_for_protocol_version(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,21 @@ 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 { + if version > ProtocolVersion::MAX + 2 { + eprintln!( + "[{}]: The network is using protocol version {:?}, which is newer than this binary; \ + the system packages used for compilation (e.g. MoveStdlib) may be out of date.", + "warning".bold().yellow(), + 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(); From 6b330076861dc5191690ebbd75284ad42e01c691 Mon Sep 17 00:00:00 2001 From: Michael George Date: Thu, 20 Feb 2025 17:26:45 -0500 Subject: [PATCH 2/5] removed dep from sui move new --- crates/sui-move/src/new.rs | 7 +----- crates/sui-source-validation/src/lib.rs | 6 +++++ .../shell_tests/new_tests/new_then_test.sh | 10 -------- ...ests__new_tests__manifest_template.sh.snap | 1 - ...l_tests__new_tests__new_then_build.sh.snap | 3 +++ ...d_bytecode_with_address_resolution.sh.snap | 10 ++++++++ ...verification_deprecation__no_flags.sh.snap | 25 +++++++++++++++++++ ...cation_deprecation__skip_dep_verif.sh.snap | 25 +++++++++++++++++++ 8 files changed, 70 insertions(+), 17 deletions(-) diff --git a/crates/sui-move/src/new.rs b/crates/sui-move/src/new.rs index d0ba1242d35d5..cf9790afa23ff 100644 --- a/crates/sui-move/src/new.rs +++ b/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()) diff --git a/crates/sui-source-validation/src/lib.rs b/crates/sui-source-validation/src/lib.rs index 8a8749ac3a7d4..0f271b906feb0 100644 --- a/crates/sui-source-validation/src/lib.rs +++ b/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; diff --git a/crates/sui/tests/shell_tests/new_tests/new_then_test.sh b/crates/sui/tests/shell_tests/new_tests/new_then_test.sh index 051d2945589a5..fe9ce4db7ff22 100644 --- a/crates/sui/tests/shell_tests/new_tests/new_then_test.sh +++ b/crates/sui/tests/shell_tests/new_tests/new_then_test.sh @@ -4,14 +4,4 @@ # check that sui move new followed by sui move test succeeds sui move new example -# we mangle the generated toml file to replace the framework dependency with a local dependency -FRAMEWORK_DIR=$(echo $CARGO_MANIFEST_DIR | sed 's#/crates/sui##g') -cat example/Move.toml \ - | sed 's#\(Sui = .*\)git = "[^"]*", \(.*\)#\1\2#' \ - | sed 's#\(Sui = .*\), rev = "[^"]*"\(.*\)#\1\2#' \ - | sed 's#\(Sui = .*\)subdir = "\([^"]*\)"\(.*\)#\1local = "FRAMEWORK/\2"\3#' \ - | sed "s#\(Sui = .*\)FRAMEWORK\(.*\)#\1$FRAMEWORK_DIR\2#" \ - > Move.toml -mv Move.toml example/Move.toml - cd example && sui move test diff --git a/crates/sui/tests/snapshots/shell_tests__new_tests__manifest_template.sh.snap b/crates/sui/tests/snapshots/shell_tests__new_tests__manifest_template.sh.snap index 64f1a3cae2ce4..175fbb7c897d9 100644 --- a/crates/sui/tests/snapshots/shell_tests__new_tests__manifest_template.sh.snap +++ b/crates/sui/tests/snapshots/shell_tests__new_tests__manifest_template.sh.snap @@ -20,7 +20,6 @@ edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move # authors = ["..."] # e.g., ["Joe Smith (joesmith@noemail.com)", "John Snow (johnsnow@noemail.com)"] [dependencies] -Sui = { git = "https://github.com/MystenLabs/sui.git", subdir = "crates/sui-framework/packages/sui-framework", rev = "framework/testnet", override = true } # For remote import, use the `{ git = "...", subdir = "...", rev = "..." }`. # Revision can be a branch, a tag, and a commit hash. diff --git a/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_build.sh.snap b/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_build.sh.snap index 06501ff6f3e5a..c0b6b1186e276 100644 --- a/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_build.sh.snap +++ b/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_build.sh.snap @@ -28,6 +28,9 @@ exit_code: 0 ----- stdout ----- ----- stderr ----- +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem INCLUDING DEPENDENCY Sui INCLUDING DEPENDENCY MoveStdlib BUILDING example diff --git a/crates/sui/tests/snapshots/shell_tests__with_network__move_build_bytecode_with_address_resolution__move_build_bytecode_with_address_resolution.sh.snap b/crates/sui/tests/snapshots/shell_tests__with_network__move_build_bytecode_with_address_resolution__move_build_bytecode_with_address_resolution.sh.snap index fc4994ae909d1..2b0989b231537 100644 --- a/crates/sui/tests/snapshots/shell_tests__with_network__move_build_bytecode_with_address_resolution__move_build_bytecode_with_address_resolution.sh.snap +++ b/crates/sui/tests/snapshots/shell_tests__with_network__move_build_bytecode_with_address_resolution__move_build_bytecode_with_address_resolution.sh.snap @@ -22,7 +22,17 @@ exit_code: 0 } ----- stderr ----- +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING simple Successfully verified dependencies on-chain against source. INCLUDING DEPENDENCY simple +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING depends_on_simple diff --git a/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__no_flags.sh.snap b/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__no_flags.sh.snap index d0017cd7750c0..af9ac007c5dcb 100644 --- a/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__no_flags.sh.snap +++ b/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__no_flags.sh.snap @@ -65,26 +65,51 @@ exit_code: 0 === munge Move.toml files === === publish dependency === [Note]: Dependency sources are no longer verified automatically during publication and upgrade. You can pass the `--verify-deps` option if you would like to verify them as part of publication or upgrade. +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING dependency Skipping dependency verification === publish package v0 (should note deprecation) === [Note]: Dependency sources are no longer verified automatically during publication and upgrade. You can pass the `--verify-deps` option if you would like to verify them as part of publication or upgrade. INCLUDING DEPENDENCY dependency +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING example Skipping dependency verification === upgrade package (should note deprecation) === [Note]: Dependency sources are no longer verified automatically during publication and upgrade. You can pass the `--verify-deps` option if you would like to verify them as part of publication or upgrade. INCLUDING DEPENDENCY dependency +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING example Skipping dependency verification === modify dependency === === try to publish with modified dep (should succeed) === [Note]: Dependency sources are no longer verified automatically during publication and upgrade. You can pass the `--verify-deps` option if you would like to verify them as part of publication or upgrade. INCLUDING DEPENDENCY dependency +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING example Skipping dependency verification === try to upgrade with modified dep (should succeed) === [Note]: Dependency sources are no longer verified automatically during publication and upgrade. You can pass the `--verify-deps` option if you would like to verify them as part of publication or upgrade. INCLUDING DEPENDENCY dependency +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING example Skipping dependency verification diff --git a/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__skip_dep_verif.sh.snap b/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__skip_dep_verif.sh.snap index dcfb6f87af9ea..9390b8551185f 100644 --- a/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__skip_dep_verif.sh.snap +++ b/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__skip_dep_verif.sh.snap @@ -65,26 +65,51 @@ exit_code: 0 === munge Move.toml files === === publish dependency (should warn about deprecation) === [Warning]: Dependency sources are no longer verified automatically during publication and upgrade, so the `--skip-dependency-verification` flag is no longer necessary. +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING dependency Skipping dependency verification === publish package v0 (should warn about deprecation) === [Warning]: Dependency sources are no longer verified automatically during publication and upgrade, so the `--skip-dependency-verification` flag is no longer necessary. INCLUDING DEPENDENCY dependency +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING example Skipping dependency verification === upgrade package (should warn about deprecation) === [Warning]: Dependency sources are no longer verified automatically during publication and upgrade, so the `--skip-dependency-verification` flag is no longer necessary. INCLUDING DEPENDENCY dependency +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING example Skipping dependency verification === modify dependency === === try to publish with modified dep (should succeed) === [Warning]: Dependency sources are no longer verified automatically during publication and upgrade, so the `--skip-dependency-verification` flag is no longer necessary. INCLUDING DEPENDENCY dependency +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING example Skipping dependency verification === try to upgrade with modified dep (should succeed) === [Warning]: Dependency sources are no longer verified automatically during publication and upgrade, so the `--skip-dependency-verification` flag is no longer necessary. INCLUDING DEPENDENCY dependency +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING example Skipping dependency verification From 505f09aac2808b6f466f8db6da9b8d3fe8a15099 Mon Sep 17 00:00:00 2001 From: Michael George Date: Fri, 21 Feb 2025 14:38:16 -0500 Subject: [PATCH 3/5] Including implicit deps for `sui move test` --- crates/sui-move/src/unit_test.rs | 6 ++++-- .../dependency/Move.toml | 3 +-- .../example/Move.toml | 2 +- .../shell_tests__new_tests__new_then_test.sh.snap | 13 +++---------- ...rce_verification_deprecation__no_flags.sh.snap | 15 --------------- ...rification_deprecation__skip_dep_verif.sh.snap | 15 --------------- ...rification_deprecation__with_dep_verif.sh.snap | 10 ++++++++++ 7 files changed, 19 insertions(+), 45 deletions(-) diff --git a/crates/sui-move/src/unit_test.rs b/crates/sui-move/src/unit_test.rs index 6d9afd93b35e5..9b37cb86df0cf 100644 --- a/crates/sui-move/src/unit_test.rs +++ b/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, 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, diff --git a/crates/sui/tests/shell_tests/with_network/source_verification_deprecation/dependency/Move.toml b/crates/sui/tests/shell_tests/with_network/source_verification_deprecation/dependency/Move.toml index bdaa4f5cb5000..bacb0b76d84ea 100644 --- a/crates/sui/tests/shell_tests/with_network/source_verification_deprecation/dependency/Move.toml +++ b/crates/sui/tests/shell_tests/with_network/source_verification_deprecation/dependency/Move.toml @@ -3,8 +3,7 @@ name = "dependency" edition = "2024.beta" [dependencies] -# Sui = { local = "FRAMEWORK_DIR", override = true } +Sui = { local = "FRAMEWORK_DIR", override = true } [addresses] dependency = "0x0" - diff --git a/crates/sui/tests/shell_tests/with_network/source_verification_deprecation/example/Move.toml b/crates/sui/tests/shell_tests/with_network/source_verification_deprecation/example/Move.toml index 75849203259ac..7c64b4d6eddd0 100644 --- a/crates/sui/tests/shell_tests/with_network/source_verification_deprecation/example/Move.toml +++ b/crates/sui/tests/shell_tests/with_network/source_verification_deprecation/example/Move.toml @@ -3,7 +3,7 @@ name = "example" edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move [dependencies] -# Sui = { local = "FRAMEWORK_DIR" } +Sui = { local = "FRAMEWORK_DIR" } dependency = { local = "../dependency" } [addresses] diff --git a/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_test.sh.snap b/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_test.sh.snap index e70031835eb4f..38fc444f8515b 100644 --- a/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_test.sh.snap +++ b/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_test.sh.snap @@ -9,22 +9,15 @@ description: tests/shell_tests/new_tests/new_then_test.sh # check that sui move new followed by sui move test succeeds sui move new example -# we mangle the generated toml file to replace the framework dependency with a local dependency -FRAMEWORK_DIR=$(echo $CARGO_MANIFEST_DIR | sed 's#/crates/sui##g') -cat example/Move.toml \ - | sed 's#\(Sui = .*\)git = "[^"]*", \(.*\)#\1\2#' \ - | sed 's#\(Sui = .*\), rev = "[^"]*"\(.*\)#\1\2#' \ - | sed 's#\(Sui = .*\)subdir = "\([^"]*\)"\(.*\)#\1local = "FRAMEWORK/\2"\3#' \ - | sed "s#\(Sui = .*\)FRAMEWORK\(.*\)#\1$FRAMEWORK_DIR\2#" \ - > Move.toml -mv Move.toml example/Move.toml - cd example && sui move test ----- results ----- success: true exit_code: 0 ----- stdout ----- +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem INCLUDING DEPENDENCY Sui INCLUDING DEPENDENCY MoveStdlib BUILDING example diff --git a/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__no_flags.sh.snap b/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__no_flags.sh.snap index af9ac007c5dcb..7df98d9c05118 100644 --- a/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__no_flags.sh.snap +++ b/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__no_flags.sh.snap @@ -65,9 +65,6 @@ exit_code: 0 === munge Move.toml files === === publish dependency === [Note]: Dependency sources are no longer verified automatically during publication and upgrade. You can pass the `--verify-deps` option if you would like to verify them as part of publication or upgrade. -INCLUDING DEPENDENCY Bridge -INCLUDING DEPENDENCY DeepBook -INCLUDING DEPENDENCY SuiSystem INCLUDING DEPENDENCY Sui INCLUDING DEPENDENCY MoveStdlib BUILDING dependency @@ -75,9 +72,6 @@ Skipping dependency verification === publish package v0 (should note deprecation) === [Note]: Dependency sources are no longer verified automatically during publication and upgrade. You can pass the `--verify-deps` option if you would like to verify them as part of publication or upgrade. INCLUDING DEPENDENCY dependency -INCLUDING DEPENDENCY Bridge -INCLUDING DEPENDENCY DeepBook -INCLUDING DEPENDENCY SuiSystem INCLUDING DEPENDENCY Sui INCLUDING DEPENDENCY MoveStdlib BUILDING example @@ -85,9 +79,6 @@ Skipping dependency verification === upgrade package (should note deprecation) === [Note]: Dependency sources are no longer verified automatically during publication and upgrade. You can pass the `--verify-deps` option if you would like to verify them as part of publication or upgrade. INCLUDING DEPENDENCY dependency -INCLUDING DEPENDENCY Bridge -INCLUDING DEPENDENCY DeepBook -INCLUDING DEPENDENCY SuiSystem INCLUDING DEPENDENCY Sui INCLUDING DEPENDENCY MoveStdlib BUILDING example @@ -96,9 +87,6 @@ Skipping dependency verification === try to publish with modified dep (should succeed) === [Note]: Dependency sources are no longer verified automatically during publication and upgrade. You can pass the `--verify-deps` option if you would like to verify them as part of publication or upgrade. INCLUDING DEPENDENCY dependency -INCLUDING DEPENDENCY Bridge -INCLUDING DEPENDENCY DeepBook -INCLUDING DEPENDENCY SuiSystem INCLUDING DEPENDENCY Sui INCLUDING DEPENDENCY MoveStdlib BUILDING example @@ -106,9 +94,6 @@ Skipping dependency verification === try to upgrade with modified dep (should succeed) === [Note]: Dependency sources are no longer verified automatically during publication and upgrade. You can pass the `--verify-deps` option if you would like to verify them as part of publication or upgrade. INCLUDING DEPENDENCY dependency -INCLUDING DEPENDENCY Bridge -INCLUDING DEPENDENCY DeepBook -INCLUDING DEPENDENCY SuiSystem INCLUDING DEPENDENCY Sui INCLUDING DEPENDENCY MoveStdlib BUILDING example diff --git a/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__skip_dep_verif.sh.snap b/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__skip_dep_verif.sh.snap index 9390b8551185f..e41b4a1b17772 100644 --- a/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__skip_dep_verif.sh.snap +++ b/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__skip_dep_verif.sh.snap @@ -65,9 +65,6 @@ exit_code: 0 === munge Move.toml files === === publish dependency (should warn about deprecation) === [Warning]: Dependency sources are no longer verified automatically during publication and upgrade, so the `--skip-dependency-verification` flag is no longer necessary. -INCLUDING DEPENDENCY Bridge -INCLUDING DEPENDENCY DeepBook -INCLUDING DEPENDENCY SuiSystem INCLUDING DEPENDENCY Sui INCLUDING DEPENDENCY MoveStdlib BUILDING dependency @@ -75,9 +72,6 @@ Skipping dependency verification === publish package v0 (should warn about deprecation) === [Warning]: Dependency sources are no longer verified automatically during publication and upgrade, so the `--skip-dependency-verification` flag is no longer necessary. INCLUDING DEPENDENCY dependency -INCLUDING DEPENDENCY Bridge -INCLUDING DEPENDENCY DeepBook -INCLUDING DEPENDENCY SuiSystem INCLUDING DEPENDENCY Sui INCLUDING DEPENDENCY MoveStdlib BUILDING example @@ -85,9 +79,6 @@ Skipping dependency verification === upgrade package (should warn about deprecation) === [Warning]: Dependency sources are no longer verified automatically during publication and upgrade, so the `--skip-dependency-verification` flag is no longer necessary. INCLUDING DEPENDENCY dependency -INCLUDING DEPENDENCY Bridge -INCLUDING DEPENDENCY DeepBook -INCLUDING DEPENDENCY SuiSystem INCLUDING DEPENDENCY Sui INCLUDING DEPENDENCY MoveStdlib BUILDING example @@ -96,9 +87,6 @@ Skipping dependency verification === try to publish with modified dep (should succeed) === [Warning]: Dependency sources are no longer verified automatically during publication and upgrade, so the `--skip-dependency-verification` flag is no longer necessary. INCLUDING DEPENDENCY dependency -INCLUDING DEPENDENCY Bridge -INCLUDING DEPENDENCY DeepBook -INCLUDING DEPENDENCY SuiSystem INCLUDING DEPENDENCY Sui INCLUDING DEPENDENCY MoveStdlib BUILDING example @@ -106,9 +94,6 @@ Skipping dependency verification === try to upgrade with modified dep (should succeed) === [Warning]: Dependency sources are no longer verified automatically during publication and upgrade, so the `--skip-dependency-verification` flag is no longer necessary. INCLUDING DEPENDENCY dependency -INCLUDING DEPENDENCY Bridge -INCLUDING DEPENDENCY DeepBook -INCLUDING DEPENDENCY SuiSystem INCLUDING DEPENDENCY Sui INCLUDING DEPENDENCY MoveStdlib BUILDING example diff --git a/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__with_dep_verif.sh.snap b/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__with_dep_verif.sh.snap index 99298f46778d5..133331ffca651 100644 --- a/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__with_dep_verif.sh.snap +++ b/crates/sui/tests/snapshots/shell_tests__with_network__source_verification_deprecation__with_dep_verif.sh.snap @@ -71,20 +71,30 @@ Fix this by rebuilding your packages with source versions matching on-chain vers ----- stderr ----- === munge Move.toml files === === publish dependency === +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING dependency Successfully verified dependencies on-chain against source. === publish package v0 (should NOT warn) === INCLUDING DEPENDENCY dependency +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING example Successfully verified dependencies on-chain against source. === upgrade package (should NOT warn) === INCLUDING DEPENDENCY dependency +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING example Successfully verified dependencies on-chain against source. === modify dependency === === try to publish with modified dep (should fail) === INCLUDING DEPENDENCY dependency +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING example === try to upgrade with modified dep (should fail) === INCLUDING DEPENDENCY dependency +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib BUILDING example From 903f9b8fed8bdb5218f3b59f4c31145aabc88345 Mon Sep 17 00:00:00 2001 From: Michael George Date: Mon, 24 Feb 2025 17:25:01 -0500 Subject: [PATCH 4/5] added tests that publish against devnet, mainnet, testnet and check implicit dependencies --- .../data/module_dependency_invalid/Move.toml | 3 + .../module_dependency_nonexistent/Move.toml | 3 + .../module_dependency_unpublished/Move.toml | 3 + .../Move.toml | 3 + .../sui/tests/shell_tests/implicits/build.sh | 5 ++ .../tests/shell_tests/implicits/build_dev.sh | 5 ++ .../shell_tests/implicits/example/Move.toml | 6 ++ .../implicits/example/sources/example.move | 10 +++ .../sui/tests/shell_tests/implicits/test.sh | 5 ++ .../tests/shell_tests/implicits/test_dev.sh | 5 ++ .../shell_tests/new_tests/new_then_build.sh | 11 --- .../shell_tests/new_tests/new_then_test.sh | 1 - .../with_network/implicits/example/Move.toml | 6 ++ .../implicits/example/sources/example.move | 10 +++ .../implicits/pub_with_implicit_deps.sh | 18 +++++ .../new_tests/new_then_publish.sh | 31 ++++++++ .../shell_tests__implicits__build.sh.snap | 17 +++++ .../shell_tests__implicits__build_dev.sh.snap | 17 +++++ .../shell_tests__implicits__test.sh.snap | 25 ++++++ .../shell_tests__implicits__test_dev.sh.snap | 25 ++++++ ...l_tests__new_tests__new_then_build.sh.snap | 11 --- ...ll_tests__new_tests__new_then_test.sh.snap | 1 - ..._implicits__pub_with_implicit_deps.sh.snap | 59 ++++++++++++++ ...twork__new_tests__new_then_publish.sh.snap | 76 +++++++++++++++++++ 24 files changed, 332 insertions(+), 24 deletions(-) create mode 100644 crates/sui/tests/shell_tests/implicits/build.sh create mode 100644 crates/sui/tests/shell_tests/implicits/build_dev.sh create mode 100644 crates/sui/tests/shell_tests/implicits/example/Move.toml create mode 100644 crates/sui/tests/shell_tests/implicits/example/sources/example.move create mode 100644 crates/sui/tests/shell_tests/implicits/test.sh create mode 100644 crates/sui/tests/shell_tests/implicits/test_dev.sh create mode 100644 crates/sui/tests/shell_tests/with_network/implicits/example/Move.toml create mode 100644 crates/sui/tests/shell_tests/with_network/implicits/example/sources/example.move create mode 100644 crates/sui/tests/shell_tests/with_network/implicits/pub_with_implicit_deps.sh create mode 100644 crates/sui/tests/shell_tests/with_network/new_tests/new_then_publish.sh create mode 100644 crates/sui/tests/snapshots/shell_tests__implicits__build.sh.snap create mode 100644 crates/sui/tests/snapshots/shell_tests__implicits__build_dev.sh.snap create mode 100644 crates/sui/tests/snapshots/shell_tests__implicits__test.sh.snap create mode 100644 crates/sui/tests/snapshots/shell_tests__implicits__test_dev.sh.snap create mode 100644 crates/sui/tests/snapshots/shell_tests__with_network__implicits__pub_with_implicit_deps.sh.snap create mode 100644 crates/sui/tests/snapshots/shell_tests__with_network__new_tests__new_then_publish.sh.snap diff --git a/crates/sui/tests/data/module_dependency_invalid/Move.toml b/crates/sui/tests/data/module_dependency_invalid/Move.toml index c7d3a5b88cde4..85a21ed1f7a94 100644 --- a/crates/sui/tests/data/module_dependency_invalid/Move.toml +++ b/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" diff --git a/crates/sui/tests/data/module_dependency_nonexistent/Move.toml b/crates/sui/tests/data/module_dependency_nonexistent/Move.toml index f9d17992d48a5..1155f472374cd 100644 --- a/crates/sui/tests/data/module_dependency_nonexistent/Move.toml +++ b/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" diff --git a/crates/sui/tests/data/module_dependency_unpublished/Move.toml b/crates/sui/tests/data/module_dependency_unpublished/Move.toml index 9261cb1cb05f9..8160a0fea3009 100644 --- a/crates/sui/tests/data/module_dependency_unpublished/Move.toml +++ b/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" diff --git a/crates/sui/tests/data/module_dependency_unpublished_non_zero_address/Move.toml b/crates/sui/tests/data/module_dependency_unpublished_non_zero_address/Move.toml index 4f23d8bbd2a7d..7ed9271e60bf4 100644 --- a/crates/sui/tests/data/module_dependency_unpublished_non_zero_address/Move.toml +++ b/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" diff --git a/crates/sui/tests/shell_tests/implicits/build.sh b/crates/sui/tests/shell_tests/implicits/build.sh new file mode 100644 index 0000000000000..f0fa24d803417 --- /dev/null +++ b/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 diff --git a/crates/sui/tests/shell_tests/implicits/build_dev.sh b/crates/sui/tests/shell_tests/implicits/build_dev.sh new file mode 100644 index 0000000000000..a669e569e54c0 --- /dev/null +++ b/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 diff --git a/crates/sui/tests/shell_tests/implicits/example/Move.toml b/crates/sui/tests/shell_tests/implicits/example/Move.toml new file mode 100644 index 0000000000000..121ad307156b3 --- /dev/null +++ b/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" diff --git a/crates/sui/tests/shell_tests/implicits/example/sources/example.move b/crates/sui/tests/shell_tests/implicits/example/sources/example.move new file mode 100644 index 0000000000000..c5c6ca83f870b --- /dev/null +++ b/crates/sui/tests/shell_tests/implicits/example/sources/example.move @@ -0,0 +1,10 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +module example::example; + +use bridge::bridge; + +public fun bridge_update_node_url(bridge: &mut bridge::Bridge, new_url: vector, ctx: &TxContext) { + bridge::update_node_url(bridge, new_url, ctx) +} diff --git a/crates/sui/tests/shell_tests/implicits/test.sh b/crates/sui/tests/shell_tests/implicits/test.sh new file mode 100644 index 0000000000000..e08e78514935d --- /dev/null +++ b/crates/sui/tests/shell_tests/implicits/test.sh @@ -0,0 +1,5 @@ +# Copyright (c) Mysten Labs, Inc. +# SPDX-License-Identifier: Apache-2.0 + +# checks that testing a package that implicitly depends on `Bridge` works +sui move test -p example 2> /dev/null diff --git a/crates/sui/tests/shell_tests/implicits/test_dev.sh b/crates/sui/tests/shell_tests/implicits/test_dev.sh new file mode 100644 index 0000000000000..dea952ae0d095 --- /dev/null +++ b/crates/sui/tests/shell_tests/implicits/test_dev.sh @@ -0,0 +1,5 @@ +# Copyright (c) Mysten Labs, Inc. +# SPDX-License-Identifier: Apache-2.0 + +# checks that testing a package with `--dev` that implicitly depends on `Bridge` works +sui move test -p example --dev 2> /dev/null diff --git a/crates/sui/tests/shell_tests/new_tests/new_then_build.sh b/crates/sui/tests/shell_tests/new_tests/new_then_build.sh index 26ef628b91151..7ade3741f587f 100644 --- a/crates/sui/tests/shell_tests/new_tests/new_then_build.sh +++ b/crates/sui/tests/shell_tests/new_tests/new_then_build.sh @@ -4,15 +4,4 @@ # tests that sui move new followed by sui move build succeeds sui move new example - -# we mangle the generated toml file to replace the framework dependency with a local dependency -FRAMEWORK_DIR=$(echo $CARGO_MANIFEST_DIR | sed 's#/crates/sui##g') -cat example/Move.toml \ - | sed 's#\(Sui = .*\)git = "[^"]*", \(.*\)#\1\2#' \ - | sed 's#\(Sui = .*\), rev = "[^"]*"\(.*\)#\1\2#' \ - | sed 's#\(Sui = .*\)subdir = "\([^"]*\)"\(.*\)#\1local = "FRAMEWORK/\2"\3#' \ - | sed "s#\(Sui = .*\)FRAMEWORK\(.*\)#\1$FRAMEWORK_DIR\2#" \ - > Move.toml -mv Move.toml example/Move.toml - cd example && sui move build diff --git a/crates/sui/tests/shell_tests/new_tests/new_then_test.sh b/crates/sui/tests/shell_tests/new_tests/new_then_test.sh index fe9ce4db7ff22..e247d83ed4f24 100644 --- a/crates/sui/tests/shell_tests/new_tests/new_then_test.sh +++ b/crates/sui/tests/shell_tests/new_tests/new_then_test.sh @@ -3,5 +3,4 @@ # check that sui move new followed by sui move test succeeds sui move new example - cd example && sui move test diff --git a/crates/sui/tests/shell_tests/with_network/implicits/example/Move.toml b/crates/sui/tests/shell_tests/with_network/implicits/example/Move.toml new file mode 100644 index 0000000000000..121ad307156b3 --- /dev/null +++ b/crates/sui/tests/shell_tests/with_network/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" diff --git a/crates/sui/tests/shell_tests/with_network/implicits/example/sources/example.move b/crates/sui/tests/shell_tests/with_network/implicits/example/sources/example.move new file mode 100644 index 0000000000000..c5c6ca83f870b --- /dev/null +++ b/crates/sui/tests/shell_tests/with_network/implicits/example/sources/example.move @@ -0,0 +1,10 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +module example::example; + +use bridge::bridge; + +public fun bridge_update_node_url(bridge: &mut bridge::Bridge, new_url: vector, ctx: &TxContext) { + bridge::update_node_url(bridge, new_url, ctx) +} diff --git a/crates/sui/tests/shell_tests/with_network/implicits/pub_with_implicit_deps.sh b/crates/sui/tests/shell_tests/with_network/implicits/pub_with_implicit_deps.sh new file mode 100644 index 0000000000000..18b68ef4af660 --- /dev/null +++ b/crates/sui/tests/shell_tests/with_network/implicits/pub_with_implicit_deps.sh @@ -0,0 +1,18 @@ +# Copyright (c) Mysten Labs, Inc. +# SPDX-License-Identifier: Apache-2.0 + +# tests that publishing a package with an implicit dependency on `Bridge` succeeds + +echo "=== set up networks ===" | tee /dev/stderr +sui client --client.config $CONFIG new-env --alias devnet --rpc https://fullnode.devnet.sui.io:443 +sui client --client.config $CONFIG new-env --alias testnet --rpc https://fullnode.testnet.sui.io:443 +sui client --client.config $CONFIG new-env --alias mainnet --rpc https://fullnode.mainnet.sui.io:443 + +for i in localnet devnet testnet mainnet; do + echo "=== publish package ($i) ===" | tee /dev/stderr + sui client --client.config $CONFIG switch --env "$i" \ + 2> /dev/null + sui client --client.config $CONFIG publish "example" \ + --dry-run \ + --json 2> /dev/null | jq '.effects.status' +done diff --git a/crates/sui/tests/shell_tests/with_network/new_tests/new_then_publish.sh b/crates/sui/tests/shell_tests/with_network/new_tests/new_then_publish.sh new file mode 100644 index 0000000000000..f439f0fbbeca6 --- /dev/null +++ b/crates/sui/tests/shell_tests/with_network/new_tests/new_then_publish.sh @@ -0,0 +1,31 @@ +# Copyright (c) Mysten Labs, Inc. +# SPDX-License-Identifier: Apache-2.0 + +# tests that sui move new followed by sui move publish succeeds on network defined by current branch + +sui move new example +echo "module example::example;" >> example/sources/example.move + +echo "=== publish package (localnet) ===" | tee /dev/stderr +sui client --client.config $CONFIG publish "example" \ + --json 2> /dev/null > output +cat output | jq '.effects.status' +UPGRADE_CAP=$(cat output | jq -r '.objectChanges[] | select(.objectType == "0x2::package::UpgradeCap") | .objectId') + +echo "=== upgrade package (localnet) ===" | tee /dev/stderr +sui client --client.config $CONFIG upgrade --upgrade-capability $UPGRADE_CAP example \ + --json 2> /dev/null | jq '.effects.status' + +echo "=== set up networks ===" | tee /dev/stderr +sui client --client.config $CONFIG new-env --alias devnet --rpc https://fullnode.devnet.sui.io:443 +sui client --client.config $CONFIG new-env --alias testnet --rpc https://fullnode.testnet.sui.io:443 +sui client --client.config $CONFIG new-env --alias mainnet --rpc https://fullnode.mainnet.sui.io:443 + +for i in devnet testnet mainnet; do + echo "=== publish package ($i) ===" | tee /dev/stderr + sui client --client.config $CONFIG switch --env "$i" \ + 2> /dev/null + sui client --client.config $CONFIG publish "example" \ + --dry-run \ + --json 2> /dev/null | jq '.effects.status' +done diff --git a/crates/sui/tests/snapshots/shell_tests__implicits__build.sh.snap b/crates/sui/tests/snapshots/shell_tests__implicits__build.sh.snap new file mode 100644 index 0000000000000..205f7b0688de9 --- /dev/null +++ b/crates/sui/tests/snapshots/shell_tests__implicits__build.sh.snap @@ -0,0 +1,17 @@ +--- +source: crates/sui/tests/shell_tests.rs +description: tests/shell_tests/implicits/build.sh +--- +----- script ----- +# 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 + +----- results ----- +success: true +exit_code: 0 +----- stdout ----- + +----- stderr ----- diff --git a/crates/sui/tests/snapshots/shell_tests__implicits__build_dev.sh.snap b/crates/sui/tests/snapshots/shell_tests__implicits__build_dev.sh.snap new file mode 100644 index 0000000000000..9a31f091d9b83 --- /dev/null +++ b/crates/sui/tests/snapshots/shell_tests__implicits__build_dev.sh.snap @@ -0,0 +1,17 @@ +--- +source: crates/sui/tests/shell_tests.rs +description: tests/shell_tests/implicits/build_dev.sh +--- +----- script ----- +# 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 + +----- results ----- +success: true +exit_code: 0 +----- stdout ----- + +----- stderr ----- diff --git a/crates/sui/tests/snapshots/shell_tests__implicits__test.sh.snap b/crates/sui/tests/snapshots/shell_tests__implicits__test.sh.snap new file mode 100644 index 0000000000000..f57cc011553b5 --- /dev/null +++ b/crates/sui/tests/snapshots/shell_tests__implicits__test.sh.snap @@ -0,0 +1,25 @@ +--- +source: crates/sui/tests/shell_tests.rs +description: tests/shell_tests/implicits/test.sh +--- +----- script ----- +# Copyright (c) Mysten Labs, Inc. +# SPDX-License-Identifier: Apache-2.0 + +# checks that testing a package that implicitly depends on `Bridge` works +sui move test -p example 2> /dev/null + +----- results ----- +success: true +exit_code: 0 +----- stdout ----- +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib +BUILDING example +Running Move unit tests +Test result: OK. Total tests: 0; passed: 0; failed: 0 + +----- stderr ----- diff --git a/crates/sui/tests/snapshots/shell_tests__implicits__test_dev.sh.snap b/crates/sui/tests/snapshots/shell_tests__implicits__test_dev.sh.snap new file mode 100644 index 0000000000000..2813995e03f9f --- /dev/null +++ b/crates/sui/tests/snapshots/shell_tests__implicits__test_dev.sh.snap @@ -0,0 +1,25 @@ +--- +source: crates/sui/tests/shell_tests.rs +description: tests/shell_tests/implicits/test_dev.sh +--- +----- script ----- +# Copyright (c) Mysten Labs, Inc. +# SPDX-License-Identifier: Apache-2.0 + +# checks that testing a package with `--dev` that implicitly depends on `Bridge` works +sui move test -p example --dev 2> /dev/null + +----- results ----- +success: true +exit_code: 0 +----- stdout ----- +INCLUDING DEPENDENCY Bridge +INCLUDING DEPENDENCY DeepBook +INCLUDING DEPENDENCY SuiSystem +INCLUDING DEPENDENCY Sui +INCLUDING DEPENDENCY MoveStdlib +BUILDING example +Running Move unit tests +Test result: OK. Total tests: 0; passed: 0; failed: 0 + +----- stderr ----- diff --git a/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_build.sh.snap b/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_build.sh.snap index c0b6b1186e276..56234681aa994 100644 --- a/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_build.sh.snap +++ b/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_build.sh.snap @@ -9,17 +9,6 @@ description: tests/shell_tests/new_tests/new_then_build.sh # tests that sui move new followed by sui move build succeeds sui move new example - -# we mangle the generated toml file to replace the framework dependency with a local dependency -FRAMEWORK_DIR=$(echo $CARGO_MANIFEST_DIR | sed 's#/crates/sui##g') -cat example/Move.toml \ - | sed 's#\(Sui = .*\)git = "[^"]*", \(.*\)#\1\2#' \ - | sed 's#\(Sui = .*\), rev = "[^"]*"\(.*\)#\1\2#' \ - | sed 's#\(Sui = .*\)subdir = "\([^"]*\)"\(.*\)#\1local = "FRAMEWORK/\2"\3#' \ - | sed "s#\(Sui = .*\)FRAMEWORK\(.*\)#\1$FRAMEWORK_DIR\2#" \ - > Move.toml -mv Move.toml example/Move.toml - cd example && sui move build ----- results ----- diff --git a/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_test.sh.snap b/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_test.sh.snap index 38fc444f8515b..2af9fe1df8519 100644 --- a/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_test.sh.snap +++ b/crates/sui/tests/snapshots/shell_tests__new_tests__new_then_test.sh.snap @@ -8,7 +8,6 @@ description: tests/shell_tests/new_tests/new_then_test.sh # check that sui move new followed by sui move test succeeds sui move new example - cd example && sui move test ----- results ----- diff --git a/crates/sui/tests/snapshots/shell_tests__with_network__implicits__pub_with_implicit_deps.sh.snap b/crates/sui/tests/snapshots/shell_tests__with_network__implicits__pub_with_implicit_deps.sh.snap new file mode 100644 index 0000000000000..979ee9b181036 --- /dev/null +++ b/crates/sui/tests/snapshots/shell_tests__with_network__implicits__pub_with_implicit_deps.sh.snap @@ -0,0 +1,59 @@ +--- +source: crates/sui/tests/shell_tests.rs +description: tests/shell_tests/with_network/implicits/pub_with_implicit_deps.sh +--- +----- script ----- +# Copyright (c) Mysten Labs, Inc. +# SPDX-License-Identifier: Apache-2.0 + +# tests that publishing a package with an implicit dependency on `Bridge` succeeds + +echo "=== set up networks ===" | tee /dev/stderr +sui client --client.config $CONFIG new-env --alias devnet --rpc https://fullnode.devnet.sui.io:443 +sui client --client.config $CONFIG new-env --alias testnet --rpc https://fullnode.testnet.sui.io:443 +sui client --client.config $CONFIG new-env --alias mainnet --rpc https://fullnode.mainnet.sui.io:443 + +for i in localnet devnet testnet mainnet; do + echo "=== publish package ($i) ===" | tee /dev/stderr + sui client --client.config $CONFIG switch --env "$i" \ + 2> /dev/null + sui client --client.config $CONFIG publish "example" \ + --dry-run \ + --json 2> /dev/null | jq '.effects.status' +done + +----- results ----- +success: true +exit_code: 0 +----- stdout ----- +=== set up networks === +Added new Sui env [devnet] to config. +Added new Sui env [testnet] to config. +Added new Sui env [mainnet] to config. +=== publish package (localnet) === +Active environment switched to [localnet] +{ + "status": "success" +} +=== publish package (devnet) === +Active environment switched to [devnet] +{ + "status": "success" +} +=== publish package (testnet) === +Active environment switched to [testnet] +{ + "status": "success" +} +=== publish package (mainnet) === +Active environment switched to [mainnet] +{ + "status": "success" +} + +----- stderr ----- +=== set up networks === +=== publish package (localnet) === +=== publish package (devnet) === +=== publish package (testnet) === +=== publish package (mainnet) === diff --git a/crates/sui/tests/snapshots/shell_tests__with_network__new_tests__new_then_publish.sh.snap b/crates/sui/tests/snapshots/shell_tests__with_network__new_tests__new_then_publish.sh.snap new file mode 100644 index 0000000000000..6b0377733fef3 --- /dev/null +++ b/crates/sui/tests/snapshots/shell_tests__with_network__new_tests__new_then_publish.sh.snap @@ -0,0 +1,76 @@ +--- +source: crates/sui/tests/shell_tests.rs +description: tests/shell_tests/with_network/new_tests/new_then_publish.sh +--- +----- script ----- +# Copyright (c) Mysten Labs, Inc. +# SPDX-License-Identifier: Apache-2.0 + +# tests that sui move new followed by sui move publish succeeds on network defined by current branch + +sui move new example +echo "module example::example;" >> example/sources/example.move + +echo "=== publish package (localnet) ===" | tee /dev/stderr +sui client --client.config $CONFIG publish "example" \ + --json 2> /dev/null > output +cat output | jq '.effects.status' +UPGRADE_CAP=$(cat output | jq -r '.objectChanges[] | select(.objectType == "0x2::package::UpgradeCap") | .objectId') + +echo "=== upgrade package (localnet) ===" | tee /dev/stderr +sui client --client.config $CONFIG upgrade --upgrade-capability $UPGRADE_CAP example \ + --json 2> /dev/null | jq '.effects.status' + +echo "=== set up networks ===" | tee /dev/stderr +sui client --client.config $CONFIG new-env --alias devnet --rpc https://fullnode.devnet.sui.io:443 +sui client --client.config $CONFIG new-env --alias testnet --rpc https://fullnode.testnet.sui.io:443 +sui client --client.config $CONFIG new-env --alias mainnet --rpc https://fullnode.mainnet.sui.io:443 + +for i in devnet testnet mainnet; do + echo "=== publish package ($i) ===" | tee /dev/stderr + sui client --client.config $CONFIG switch --env "$i" \ + 2> /dev/null + sui client --client.config $CONFIG publish "example" \ + --dry-run \ + --json 2> /dev/null | jq '.effects.status' +done + +----- results ----- +success: true +exit_code: 0 +----- stdout ----- +=== publish package (localnet) === +{ + "status": "success" +} +=== upgrade package (localnet) === +{ + "status": "success" +} +=== set up networks === +Added new Sui env [devnet] to config. +Added new Sui env [testnet] to config. +Added new Sui env [mainnet] to config. +=== publish package (devnet) === +Active environment switched to [devnet] +{ + "status": "success" +} +=== publish package (testnet) === +Active environment switched to [testnet] +{ + "status": "success" +} +=== publish package (mainnet) === +Active environment switched to [mainnet] +{ + "status": "success" +} + +----- stderr ----- +=== publish package (localnet) === +=== upgrade package (localnet) === +=== set up networks === +=== publish package (devnet) === +=== publish package (testnet) === +=== publish package (mainnet) === From ff50a7d2fb9c5f1112d5e1be2f1fc87045fbc341 Mon Sep 17 00:00:00 2001 From: stefan-mysten <135084671+stefan-mysten@users.noreply.github.com> Date: Tue, 25 Feb 2025 15:36:45 -0800 Subject: [PATCH 5/5] New alg for tree shaking that uses linkage tables for determining which packages to keep for deps. --- Cargo.lock | 2 + crates/sui-move-build/src/lib.rs | 43 ++--- crates/sui-move/src/build.rs | 27 +--- crates/sui/Cargo.toml | 2 + crates/sui/src/client_commands.rs | 149 +++++++++++++++--- crates/sui/src/sui_commands.rs | 85 +++++++--- crates/sui/src/upgrade_compatibility/mod.rs | 7 +- crates/sui/tests/cli_tests.rs | 36 ++++- .../tree_shaking/J_system_deps/.gitignore | 1 + .../data/tree_shaking/J_system_deps/Move.toml | 11 ++ .../J_system_deps/sources/j_system_deps.move | 10 ++ 11 files changed, 271 insertions(+), 102 deletions(-) create mode 100644 crates/sui/tests/data/tree_shaking/J_system_deps/.gitignore create mode 100644 crates/sui/tests/data/tree_shaking/J_system_deps/Move.toml create mode 100644 crates/sui/tests/data/tree_shaking/J_system_deps/sources/j_system_deps.move diff --git a/Cargo.lock b/Cargo.lock index 38f173613a596..362e1f73aebae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12418,11 +12418,13 @@ dependencies = [ "move-binary-format", "move-bytecode-source-map", "move-bytecode-verifier-meter", + "move-cli", "move-command-line-common", "move-compiler", "move-core-types", "move-ir-types", "move-package", + "move-symbol-pool", "move-vm-config", "move-vm-profiler", "msim", diff --git a/crates/sui-move-build/src/lib.rs b/crates/sui-move-build/src/lib.rs index 85e1ee03f3864..edc0c652fb3f9 100644 --- a/crates/sui-move-build/src/lib.rs +++ b/crates/sui-move-build/src/lib.rs @@ -644,15 +644,12 @@ impl CompiledPackage { self.dependency_ids.published.values().cloned().collect() } - /// Tree-shake the package's dependencies to remove any that are not referenced in source code. - /// - /// This algorithm uses the set of root modules as the starting point to retrieve the - /// list of used packages that are immediate dependencies of these modules. Essentially, it - /// will remove any package that has no immediate module dependency to it. - /// - /// Then, it will recursively find all the transitive dependencies of the packages in the list - /// above and add them to the list of packages that need to be kept as dependencies. - pub fn tree_shake(&mut self, with_unpublished_deps: bool) -> Result<(), anyhow::Error> { + /// Find the map of packages that are immediate dependencies of the root modules, joined with + /// the set of bytecode dependencies. + pub fn find_immediate_deps_pkgs_to_keep( + &self, + with_unpublished_deps: bool, + ) -> Result, anyhow::Error> { // Start from the root modules (or all modules if with_unpublished_deps is true as we // need to include modules with 0x0 address) let root_modules: Vec<_> = if with_unpublished_deps { @@ -669,7 +666,7 @@ impl CompiledPackage { }; // Find the immediate dependencies for each root module and store the package name - // in the used_immediate_packages set. This basically prunes the packages that are not used + // in the pkgs_to_keep set. This basically prunes the packages that are not used // based on the modules information. let mut pkgs_to_keep: BTreeSet = BTreeSet::new(); let module_to_pkg_name: BTreeMap<_, _> = self @@ -677,7 +674,6 @@ impl CompiledPackage { .all_modules() .map(|m| (m.unit.module.self_id(), m.unit.package_name)) .collect(); - let mut used_immediate_packages: BTreeSet = BTreeSet::new(); for module in &root_modules { let immediate_deps = module.module.immediate_dependencies(); @@ -686,31 +682,24 @@ impl CompiledPackage { let Some(pkg_name) = pkg_name else { bail!("Expected a package name but it's None") }; - used_immediate_packages.insert(*pkg_name); + pkgs_to_keep.insert(*pkg_name); } } } - // Next, for each package from used_immediate_packages set, we need to find all the - // transitive dependencies. Those trans dependencies need to be included in the final list - // of package dependencies (note that the pkg itself will be added by the transitive deps - // function) - used_immediate_packages.iter().for_each(|pkg| { - self.dependency_graph - .add_transitive_dependencies(pkg, &mut pkgs_to_keep) - }); - // If a package depends on another published package that has only bytecode without source // code available, we need to include also that package as dep. pkgs_to_keep.extend(self.bytecode_deps.iter().map(|(name, _)| *name)); - // Finally, filter out packages that are not in the union above from the - // dependency_ids.published field and return the package ids. - self.dependency_ids + // Finally, filter out packages that are published and exist in the manifest at the + // compilation time but are not referenced in the source code. + Ok(self + .dependency_ids + .clone() .published - .retain(|pkg_name, _| pkgs_to_keep.contains(pkg_name)); - - Ok(()) + .into_iter() + .filter(|(pkg_name, _)| pkgs_to_keep.contains(pkg_name)) + .collect()) } } diff --git a/crates/sui-move/src/build.rs b/crates/sui-move/src/build.rs index 07d634b22f0ef..c43d84c6bef17 100644 --- a/crates/sui-move/src/build.rs +++ b/crates/sui-move/src/build.rs @@ -5,11 +5,8 @@ use crate::manage_package::resolve_lock_file_path; use clap::Parser; 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, implicit_deps, BuildConfig, -}; +use sui_move_build::{implicit_deps, BuildConfig}; use sui_package_management::system_package_versions::latest_system_packages; const LAYOUTS_DIR: &str = "layouts"; @@ -55,8 +52,6 @@ impl Build { Self::execute_internal( &rerooted_path, build_config, - self.with_unpublished_dependencies, - self.dump_bytecode_as_base64, self.generate_struct_layouts, self.chain_id.clone(), ) @@ -65,35 +60,17 @@ impl Build { pub fn execute_internal( rerooted_path: &Path, mut config: MoveBuildConfig, - with_unpublished_deps: bool, - dump_bytecode_as_base64: bool, generate_struct_layouts: bool, chain_id: Option, ) -> anyhow::Result<()> { config.implicit_dependencies = implicit_deps(latest_system_packages()); - let mut pkg = BuildConfig { + let pkg = BuildConfig { config, run_bytecode_verifier: true, print_diags_to_stderr: true, chain_id, } .build(rerooted_path)?; - if dump_bytecode_as_base64 { - check_invalid_dependencies(&pkg.dependency_ids.invalid)?; - if !with_unpublished_deps { - check_unpublished_dependencies(&pkg.dependency_ids.unpublished)?; - } - - pkg.tree_shake(with_unpublished_deps)?; - println!( - "{}", - json!({ - "modules": pkg.get_package_base64(with_unpublished_deps), - "dependencies": pkg.get_dependency_storage_package_ids(), - "digest": pkg.get_package_digest(with_unpublished_deps), - }) - ) - } if generate_struct_layouts { let layout_str = serde_yaml::to_string(&pkg.generate_struct_layouts()).unwrap(); diff --git a/crates/sui/Cargo.toml b/crates/sui/Cargo.toml index 37fb4a02b7c18..4215949e13e91 100644 --- a/crates/sui/Cargo.toml +++ b/crates/sui/Cargo.toml @@ -98,6 +98,8 @@ move-vm-profiler.workspace = true move-vm-config.workspace = true move-ir-types.workspace = true move-command-line-common.workspace = true +move-cli.workspace = true +move-symbol-pool.workspace = true [target.'cfg(not(target_env = "msvc"))'.dependencies] jemalloc-ctl.workspace = true diff --git a/crates/sui/src/client_commands.rs b/crates/sui/src/client_commands.rs index 9df2282efa048..085b556a47c19 100644 --- a/crates/sui/src/client_commands.rs +++ b/crates/sui/src/client_commands.rs @@ -10,7 +10,7 @@ use crate::{ verifier_meter::{AccumulatingMeter, Accumulator}, }; use std::{ - collections::{btree_map::Entry, BTreeMap}, + collections::{btree_map::Entry, BTreeMap, BTreeSet}, fmt::{Debug, Display, Formatter, Write}, fs, path::{Path, PathBuf}, @@ -70,12 +70,12 @@ use sui_types::{ base_types::{ObjectID, SequenceNumber, SuiAddress}, crypto::{EmptySignInfo, SignatureScheme}, digests::TransactionDigest, - error::SuiError, + error::{SuiError, SuiObjectResponseError}, gas::GasCostSummary, gas_coin::GasCoin, message_envelope::Envelope, metrics::BytecodeVerifierMetrics, - move_package::UpgradeCap, + move_package::{MovePackage, UpgradeCap}, object::Owner, parse_sui_type_tag, signature::GenericSignature, @@ -97,6 +97,7 @@ use tabled::{ }, }; +use move_symbol_pool::Symbol; use sui_types::digests::ChainIdentifier; use tracing::{debug, info}; @@ -893,12 +894,9 @@ impl SuiClientCommands { let sender = context.try_get_object_owner(&opts.gas).await?; let sender = sender.unwrap_or(context.active_address()?); let client = context.get_client().await?; - let chain_id = client.read_api().get_chain_identifier().await.ok(); - let protocol_version = client - .read_api() - .get_protocol_config(None) - .await? - .protocol_version; + let read_api = client.read_api(); + let chain_id = read_api.get_chain_identifier().await.ok(); + let protocol_version = read_api.get_protocol_config(None).await?.protocol_version; let protocol_config = ProtocolConfig::get_for_version( protocol_version, match chain_id @@ -910,7 +908,7 @@ impl SuiClientCommands { }, ); - check_protocol_version_and_warn(&client).await?; + check_protocol_version_and_warn(read_api).await?; let package_path = package_path .canonicalize() @@ -937,7 +935,7 @@ impl SuiClientCommands { check_dep_verification_flags(skip_dependency_verification, verify_deps)?; let upgrade_result = upgrade_package( - client.read_api(), + read_api, build_config.clone(), &package_path, upgrade_capability, @@ -969,7 +967,7 @@ impl SuiClientCommands { if verify_compatibility { check_compatibility( - &client, + read_api, package_id, compiled_package, package_path, @@ -1042,9 +1040,10 @@ impl SuiClientCommands { let sender = context.try_get_object_owner(&opts.gas).await?; let sender = sender.unwrap_or(context.active_address()?); let client = context.get_client().await?; - let chain_id = client.read_api().get_chain_identifier().await.ok(); + let read_api = client.read_api(); + let chain_id = read_api.get_chain_identifier().await.ok(); - check_protocol_version_and_warn(&client).await?; + check_protocol_version_and_warn(read_api).await?; let package_path = package_path .canonicalize() @@ -1066,7 +1065,7 @@ impl SuiClientCommands { check_dep_verification_flags(skip_dependency_verification, verify_deps)?; let compile_result = compile_package( - client.read_api(), + read_api, build_config.clone(), &package_path, with_unpublished_dependencies, @@ -1124,6 +1123,8 @@ impl SuiClientCommands { package_path, build_config, } => { + let client = context.get_client().await?; + let read_api = client.read_api(); let protocol_version = protocol_version.map_or(ProtocolVersion::MAX, ProtocolVersion::new); let protocol_config = @@ -1151,7 +1152,9 @@ impl SuiClientCommands { (_, package_path) => { let package_path = package_path.unwrap_or_else(|| PathBuf::from(".")); - let package = compile_package_simple(build_config, &package_path, None)?; + let package = + compile_package_simple(read_api, build_config, &package_path, None) + .await?; let name = package .package .compiled_package_info @@ -1766,7 +1769,8 @@ fn check_dep_verification_flags( } } -fn compile_package_simple( +async fn compile_package_simple( + read_api: &ReadApi, mut build_config: MoveBuildConfig, package_path: &Path, chain_id: Option, @@ -1781,7 +1785,7 @@ fn compile_package_simple( let resolution_graph = config.resolution_graph(package_path, chain_id.clone())?; let mut compiled_package = build_from_resolution_graph(resolution_graph, false, false, chain_id)?; - compiled_package.tree_shake(false)?; + pkg_tree_shake(read_api, false, &mut compiled_package).await?; Ok(compiled_package) } @@ -1803,7 +1807,13 @@ pub(crate) async fn upgrade_package( skip_dependency_verification, ) .await?; - compiled_package.tree_shake(with_unpublished_dependencies)?; + + pkg_tree_shake( + read_api, + with_unpublished_dependencies, + &mut compiled_package, + ) + .await?; compiled_package.published_at.as_ref().map_err(|e| match e { PublishedAtError::NotPresent => { @@ -1891,7 +1901,14 @@ pub(crate) async fn compile_package( print_diags_to_stderr, chain_id, )?; - compiled_package.tree_shake(with_unpublished_dependencies)?; + + pkg_tree_shake( + read_api, + with_unpublished_dependencies, + &mut compiled_package, + ) + .await?; + let protocol_config = read_api.get_protocol_config(None).await?; // Check that the package's Move version is compatible with the chain's @@ -3100,8 +3117,8 @@ pub(crate) async fn prerender_clever_errors( } /// Warn the user if the CLI falls behind more than 2 protocol versions. -async fn check_protocol_version_and_warn(client: &SuiClient) -> Result<(), anyhow::Error> { - let protocol_cfg = client.read_api().get_protocol_config(None).await?; +async fn check_protocol_version_and_warn(read_api: &ReadApi) -> Result<(), anyhow::Error> { + let protocol_cfg = read_api.get_protocol_config(None).await?; let on_chain_protocol_version = protocol_cfg.protocol_version.as_u64(); let cli_protocol_version = ProtocolVersion::MAX.as_u64(); if (cli_protocol_version + 2) < on_chain_protocol_version { @@ -3122,3 +3139,91 @@ async fn check_protocol_version_and_warn(client: &SuiClient) -> Result<(), anyho Ok(()) } + +/// Fetch move packages based on the provided package IDs. +async fn fetch_move_packages( + read_api: &ReadApi, + package_ids: &[ObjectID], + pkg_id_to_name: &BTreeMap<&ObjectID, &Symbol>, +) -> Result, anyhow::Error> { + let objects = read_api + .multi_get_object_with_options(package_ids.to_vec(), SuiObjectDataOptions::bcs_lossless()) + .await?; + + objects + .into_iter() + .map(|o| { + let o = o.into_object().map_err(|e| match e { + SuiObjectResponseError::NotExists { object_id } => { + anyhow!( + "Package {} with object ID {object_id} does not exist", + pkg_id_to_name.get(&object_id).unwrap() + ) + } + SuiObjectResponseError::Deleted { + object_id, + version, + digest, + } => { + anyhow!( + "Package {} with object ID {object_id} was deleted at version {version} \ + with digest {digest}", + pkg_id_to_name.get(&object_id).unwrap() + ) + } + _ => anyhow!("Cannot convert data into an object: {e}"), + })?; + + let Some(SuiRawData::Package(p)) = o.bcs else { + bail!( + "Expected package {} with object ID {} but got something else", + pkg_id_to_name.get(&o.object_id).unwrap(), + o.object_id + ); + }; + p.to_move_package(u64::MAX /* safe as this pkg comes from the network */) + .map_err(|e| anyhow!(e)) + }) + .collect() +} + +/// Filter out a package's dependencies which are not referenced in the source code. +pub(crate) async fn pkg_tree_shake( + read_api: &ReadApi, + with_unpublished_dependencies: bool, + compiled_package: &mut CompiledPackage, +) -> Result<(), anyhow::Error> { + let pkgs = compiled_package.find_immediate_deps_pkgs_to_keep(with_unpublished_dependencies)?; + let pkg_ids = pkgs.values().cloned().collect::>(); + let pkg_id_to_name = pkgs + .iter() + .map(|(name, id)| (id, name)) + .collect::>(); + + let pkg_name_to_orig_id: BTreeMap<_, _> = compiled_package + .package + .deps_compiled_units + .iter() + .map(|(pkg_name, module)| (pkg_name, ObjectID::from(module.unit.address.into_inner()))) + .collect(); + + let published_deps_packages = fetch_move_packages(read_api, &pkg_ids, &pkg_id_to_name).await?; + + let linkage_table_ids: BTreeSet<_> = published_deps_packages + .iter() + .flat_map(|pkg| pkg.linkage_table().keys()) + .collect(); + + compiled_package + .dependency_ids + .published + .retain(|pkg_name, id| { + linkage_table_ids.contains(id) + || pkgs.contains_key(pkg_name) + || pkg_name_to_orig_id + .get(pkg_name) + .is_some_and(|orig_id| pkg_ids.contains(orig_id)) + }); + + Ok(()) +} diff --git a/crates/sui/src/sui_commands.rs b/crates/sui/src/sui_commands.rs index 09a628a44d2b4..0cf861b002c84 100644 --- a/crates/sui/src/sui_commands.rs +++ b/crates/sui/src/sui_commands.rs @@ -1,7 +1,7 @@ // Copyright (c) Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 -use crate::client_commands::SuiClientCommands; +use crate::client_commands::{pkg_tree_shake, SuiClientCommands}; use crate::console::start_console; use crate::fire_drill::{run_fire_drill, FireDrill}; use crate::genesis_ceremony::{run, Ceremony}; @@ -43,10 +43,15 @@ use sui_graphql_rpc::{ test_infra::cluster::start_graphql_server_with_fn_rpc, }; +use serde_json::json; use sui_keys::keypair_file::read_key; use sui_keys::keystore::{AccountKeystore, FileBasedKeystore, Keystore}; +use sui_move::manage_package::resolve_lock_file_path; use sui_move::{self, execute_move_command}; -use sui_move_build::SuiPackageHooks; +use sui_move_build::{ + check_invalid_dependencies, check_unpublished_dependencies, BuildConfig as SuiBuildConfig, + SuiPackageHooks, +}; use sui_sdk::sui_client_config::{SuiClientConfig, SuiEnv}; use sui_sdk::wallet_context::WalletContext; use sui_swarm::memory::Swarm; @@ -499,31 +504,67 @@ impl SuiCommand { SuiCommand::Move { package_path, build_config, - mut cmd, + cmd, config: client_config, } => { - match &mut cmd { + match cmd { sui_move::Command::Build(build) if build.dump_bytecode_as_base64 => { - if build.ignore_chain { - build.chain_id = None; + // `sui move build` does not ordinarily require a network connection. + // The exception is when --dump-bytecode-as-base64 is specified: In this + // case, we should resolve the correct addresses for the respective chain + // (e.g., testnet, mainnet) from the Move.lock under automated address management. + // In addition, tree shaking also requires a network as it needs to fetch + // on-chain linkage table of package dependencies. + let config = + client_config.unwrap_or(sui_config_dir()?.join(SUI_CLIENT_CONFIG)); + prompt_if_no_config(&config, false).await?; + let context = WalletContext::new(&config, None, None)?; + + let Ok(client) = context.get_client().await else { + bail!("`sui move build --dump-bytecode-as-base64` requires a connection to the network. Current active network is {} but failed to connect to it.", context.config.active_env.as_ref().unwrap()); + }; + let read_api = client.read_api(); + + if let Err(e) = client.check_api_version() { + eprintln!("{}", format!("[warning] {e}").yellow().bold()); + } + + let chain_id = if build.ignore_chain { + // for tests it's useful to ignore the chain id! + None } else { - // `sui move build` does not ordinarily require a network connection. - // The exception is when --dump-bytecode-as-base64 is specified: In this - // case, we should resolve the correct addresses for the respective chain - // (e.g., testnet, mainnet) from the Move.lock under automated address management. - let config = - client_config.unwrap_or(sui_config_dir()?.join(SUI_CLIENT_CONFIG)); - prompt_if_no_config(&config, false).await?; - let context = WalletContext::new(&config, None, None)?; - if let Ok(client) = context.get_client().await { - if let Err(e) = client.check_api_version() { - eprintln!("{}", format!("[warning] {e}").yellow().bold()); - } - } - let client = context.get_client().await?; - let chain_id = client.read_api().get_chain_identifier().await.ok(); - build.chain_id = chain_id.clone(); + read_api.get_chain_identifier().await.ok() + }; + + let rerooted_path = move_cli::base::reroot_path(package_path.as_deref())?; + let build_config = + resolve_lock_file_path(build_config, Some(&rerooted_path))?; + let mut pkg = SuiBuildConfig { + config: build_config, + run_bytecode_verifier: true, + print_diags_to_stderr: true, + chain_id, + } + .build(&rerooted_path)?; + + let with_unpublished_deps = build.with_unpublished_dependencies; + + check_invalid_dependencies(&pkg.dependency_ids.invalid)?; + if !with_unpublished_deps { + check_unpublished_dependencies(&pkg.dependency_ids.unpublished)?; } + + pkg_tree_shake(read_api, with_unpublished_deps, &mut pkg).await?; + + println!( + "{}", + json!({ + "modules": pkg.get_package_base64(with_unpublished_deps), + "dependencies": pkg.get_dependency_storage_package_ids(), + "digest": pkg.get_package_digest(with_unpublished_deps), + }) + ); + return Ok(()); } _ => (), }; diff --git a/crates/sui/src/upgrade_compatibility/mod.rs b/crates/sui/src/upgrade_compatibility/mod.rs index f9da6d38137bc..a2853adc5f6b7 100644 --- a/crates/sui/src/upgrade_compatibility/mod.rs +++ b/crates/sui/src/upgrade_compatibility/mod.rs @@ -45,7 +45,7 @@ use move_package::compilation::compiled_package::CompiledUnitWithSource; use sui_json_rpc_types::{SuiObjectDataOptions, SuiRawData}; use sui_move_build::CompiledPackage; use sui_protocol_config::ProtocolConfig; -use sui_sdk::SuiClient; +use sui_sdk::apis::ReadApi; use sui_types::move_package::UpgradePolicy; use sui_types::{base_types::ObjectID, execution_config_utils::to_binary_config}; @@ -654,15 +654,14 @@ upgrade_codes!( /// Check the upgrade compatibility of a new package with an existing on-chain package. pub(crate) async fn check_compatibility( - client: &SuiClient, + read_api: &ReadApi, package_id: ObjectID, new_package: CompiledPackage, package_path: PathBuf, upgrade_policy: u8, protocol_config: ProtocolConfig, ) -> Result<(), Error> { - let existing_obj_read = client - .read_api() + let existing_obj_read = read_api .get_object_with_options(package_id, SuiObjectDataOptions::new().with_bcs()) .await .context("Unable to get existing package")?; diff --git a/crates/sui/tests/cli_tests.rs b/crates/sui/tests/cli_tests.rs index 95fb0a3cc3a74..29b45e4bf6365 100644 --- a/crates/sui/tests/cli_tests.rs +++ b/crates/sui/tests/cli_tests.rs @@ -104,7 +104,9 @@ impl TreeShakingTest { let temp_dir = tempfile::Builder::new().prefix("tree_shaking").tempdir()?; std::fs::create_dir_all(temp_dir.path()).unwrap(); let tests_dir = PathBuf::from(TEST_DATA_DIR); + let framework_pkgs = PathBuf::from("../sui-framework/packages"); copy_dir_all(tests_dir, temp_dir.path())?; + copy_dir_all(framework_pkgs, temp_dir.path().join("system-packages"))?; Ok(Self { test_cluster, @@ -1877,7 +1879,7 @@ async fn test_package_publish_nonexistent_dependency() -> Result<(), anyhow::Err let err = result.unwrap_err().to_string(); assert!( - err.contains("Dependency object does not exist or was deleted"), + err.contains("Package Nonexistent with object ID 0x0000000000000000000000000000000000000000000000000000000000abc123 does not exist"), "{}", err ); @@ -4330,7 +4332,7 @@ async fn test_tree_shaking_package_with_unused_dependency() -> Result<(), anyhow } #[sim_test] -async fn test_tree_shaking_package_with_transitive_dependencies() -> Result<(), anyhow::Error> { +async fn test_tree_shaking_package_with_transitive_dependencies1() -> Result<(), anyhow::Error> { let mut test = TreeShakingTest::new().await?; // Publish packages A and B @@ -4519,3 +4521,33 @@ async fn test_tree_shaking_package_deps_on_pkg_upgrade_1() -> Result<(), anyhow: Ok(()) } + +#[sim_test] +async fn test_tree_shaking_package_system_deps() -> Result<(), anyhow::Error> { + let mut test = TreeShakingTest::new().await?; + + // Publish package J and verify empty linkage table + let (package_j_id, _) = test.publish_package("J_system_deps", false).await?; + let move_pkg_j = fetch_move_packages(&test.client, vec![package_j_id]).await; + let linkage_table_j = move_pkg_j.first().unwrap().linkage_table(); + assert!( + linkage_table_j.is_empty(), + "Package J should have no dependencies" + ); + + // sui move build --dump-bytecode-as-base64 should also yield a json with no dependencies + let package_path = test.package_path("J_system_deps"); + let binary_path = env!("CARGO_BIN_EXE_sui"); + let cmd = std::process::Command::new(binary_path) + .arg("move") + .arg("build") + .arg("--dump-bytecode-as-base64") + .arg(package_path) + .output() + .expect("Failed to execute command"); + + let output = String::from_utf8_lossy(&cmd.stdout); + assert!(!output.contains("dependencies: []")); + + Ok(()) +} diff --git a/crates/sui/tests/data/tree_shaking/J_system_deps/.gitignore b/crates/sui/tests/data/tree_shaking/J_system_deps/.gitignore new file mode 100644 index 0000000000000..a007feab071f4 --- /dev/null +++ b/crates/sui/tests/data/tree_shaking/J_system_deps/.gitignore @@ -0,0 +1 @@ +build/* diff --git a/crates/sui/tests/data/tree_shaking/J_system_deps/Move.toml b/crates/sui/tests/data/tree_shaking/J_system_deps/Move.toml new file mode 100644 index 0000000000000..7cb2aee6c6f0c --- /dev/null +++ b/crates/sui/tests/data/tree_shaking/J_system_deps/Move.toml @@ -0,0 +1,11 @@ +[package] +name = "J_system_deps" +edition = "2024.beta" + +[dependencies] +# this is relative to the temp directory where this whole tree shaking dir is copied to +Sui = { local = "../../system-packages/sui-framework", override = true } + +[addresses] +j_system_deps = "0x0" + diff --git a/crates/sui/tests/data/tree_shaking/J_system_deps/sources/j_system_deps.move b/crates/sui/tests/data/tree_shaking/J_system_deps/sources/j_system_deps.move new file mode 100644 index 0000000000000..3d8f2c1560902 --- /dev/null +++ b/crates/sui/tests/data/tree_shaking/J_system_deps/sources/j_system_deps.move @@ -0,0 +1,10 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +module j_system_deps::j_system_deps { + public fun j_system_deps() { + let x = 1; + let y = 2; + let z = x + y; + } +}