[CLI] cherry pick (#20977) Deprecation warnings for dependency verifi…

…cation (#21127) ## Description This adds a warning that source verification will become opt-in instead of opt-out in a future release, along with the `--verify-deps` flag that currently disables the warning. ## Test plan Several shell tests that cover the behavior with no flags, with both flags, and with each flag independently, on a package with source that has changed since publication. See the snapshot files for the tests and expected output. ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] gRPC: - [ ] JSON-RPC: - [ ] GraphQL: - [X] CLI: publication and upgrade will now warn that source verification will become opt-in in a future release; the warning can be disabled with either `--skip-dependency-verification` or the new `--verify-deps` flags - [ ] Rust SDK:
MystenLabs · Feb 7, 2025 · 55ba9e9 · 55ba9e9
1 parent 23424e7
commit 55ba9e9
Show file tree

Hide file tree

Showing 19 changed files with 533 additions and 10 deletions.
diff --git a/crates/sui-source-validation-service/tests/tests.rs b/crates/sui-source-validation-service/tests/tests.rs
@@ -181,6 +181,7 @@ async fn run_publish(
         package_path: package_path.clone(),
         build_config,
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
     }
@@ -208,6 +209,7 @@ async fn run_upgrade(
         upgrade_capability: cap.reference.object_id,
         build_config,
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         verify_compatibility: true,

diff --git a/crates/sui/src/client_commands.rs b/crates/sui/src/client_commands.rs
@@ -360,11 +360,16 @@ pub enum SuiClientCommands {
         #[clap(flatten)]
         opts: OptsWithGas,
 
-        /// Publish the package without checking whether compiling dependencies from source results
-        /// in bytecode matching the dependencies found on-chain.
+        /// Publish the package without checking whether dependency source code compiles to the
+        /// on-chain bytecode
         #[clap(long)]
         skip_dependency_verification: bool,
 
+        /// Check that the dependency source code compiles to the on-chain bytecode before
+        /// publishing the package (currently the default behavior)
+        #[clap(long, conflicts_with = "skip_dependency_verification")]
+        verify_deps: bool,
+
         /// Also publish transitive dependencies that have not already been published.
         #[clap(long)]
         with_unpublished_dependencies: bool,
@@ -465,11 +470,16 @@ pub enum SuiClientCommands {
         #[clap(long)]
         verify_compatibility: bool,
 
-        /// Publish the package without checking whether compiling dependencies from source results
-        /// in bytecode matching the dependencies found on-chain.
+        /// Upgrade the package without checking whether dependency source code compiles to the on-chain
+        /// bytecode
         #[clap(long)]
         skip_dependency_verification: bool,
 
+        /// Check that the dependency source code compiles to the on-chain bytecode before
+        /// upgrading the package (currently the default behavior)
+        #[clap(long, conflicts_with = "skip_dependency_verification")]
+        verify_deps: bool,
+
         /// Also publish transitive dependencies that have not already been published.
         #[clap(long)]
         with_unpublished_dependencies: bool,
@@ -872,6 +882,7 @@ impl SuiClientCommands {
                 upgrade_capability,
                 build_config,
                 skip_dependency_verification,
+                verify_deps,
                 verify_compatibility,
                 with_unpublished_dependencies,
                 opts,
@@ -897,7 +908,6 @@ impl SuiClientCommands {
                 );
 
                 check_protocol_version_and_warn(&client).await?;
-
                 let package_path =
                     package_path
                         .canonicalize()
@@ -920,13 +930,16 @@ impl SuiClientCommands {
                     .get_active_env()
                     .map(|e| e.alias.clone())
                     .ok();
+                let verify =
+                    check_dep_verification_flags(skip_dependency_verification, verify_deps)?;
+
                 let upgrade_result = upgrade_package(
                     client.read_api(),
                     build_config.clone(),
                     &package_path,
                     upgrade_capability,
                     with_unpublished_dependencies,
-                    skip_dependency_verification,
+                    !verify,
                     env_alias,
                 )
                 .await;
@@ -1001,6 +1014,7 @@ impl SuiClientCommands {
                 package_path,
                 build_config,
                 skip_dependency_verification,
+                verify_deps,
                 with_unpublished_dependencies,
                 opts,
             } => {
@@ -1025,7 +1039,6 @@ impl SuiClientCommands {
                 let chain_id = client.read_api().get_chain_identifier().await.ok();
 
                 check_protocol_version_and_warn(&client).await?;
-
                 let package_path =
                     package_path
                         .canonicalize()
@@ -1043,12 +1056,15 @@ impl SuiClientCommands {
                 } else {
                     None
                 };
+                let verify =
+                    check_dep_verification_flags(skip_dependency_verification, verify_deps)?;
+
                 let compile_result = compile_package(
                     client.read_api(),
                     build_config.clone(),
                     &package_path,
                     with_unpublished_dependencies,
-                    skip_dependency_verification,
+                    !verify,
                 )
                 .await;
                 // Restore original ID, then check result.
@@ -1713,6 +1729,28 @@ impl SuiClientCommands {
     }
 }
 
+/// Process the `--skip-dependency-verification` and `--verify-dependencies` flags for a publish or
+/// upgrade command. Prints deprecation warnings as appropriate and returns true if the
+/// dependencies should be verified
+fn check_dep_verification_flags(
+    skip_dependency_verification: bool,
+    verify_dependencies: bool,
+) -> anyhow::Result<bool> {
+    match (skip_dependency_verification, verify_dependencies) {
+        (true, true) => bail!("[error]: --skip_dependency_verification and --verify_dependencies are mutually exclusive"),
+
+        (false, false) => {
+            eprintln!("{}: In a future release, dependency source code will no longer be verified by default during publication and upgrade. \
+                You can opt in to source verification using `--verify-deps` or disable this warning using `--skip-dependency-verification`. \
+                You can also manually verify dependencies using `sui client verify-source`.",
+                "[warning]".bold().yellow());
+            Ok(true)
+        },
+
+        _ => Ok(verify_dependencies),
+    }
+}
+
 fn compile_package_simple(
     build_config: MoveBuildConfig,
     package_path: &Path,

diff --git a/crates/sui/tests/cli_tests.rs b/crates/sui/tests/cli_tests.rs
@@ -245,6 +245,7 @@ async fn test_ptb_publish_and_complex_arg_resolution() -> Result<(), anyhow::Err
         package_path: package_path.clone(),
         build_config,
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
     }
@@ -524,6 +525,7 @@ async fn test_move_call_args_linter_command() -> Result<(), anyhow::Error> {
         build_config,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -788,6 +790,7 @@ async fn test_package_publish_command() -> Result<(), anyhow::Error> {
         build_config,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -858,6 +861,7 @@ async fn test_package_management_on_publish_command() -> Result<(), anyhow::Erro
         build_config: build_config.clone(),
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -928,6 +932,7 @@ async fn test_delete_shared_object() -> Result<(), anyhow::Error> {
         build_config,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -1032,6 +1037,7 @@ async fn test_receive_argument() -> Result<(), anyhow::Error> {
         build_config,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -1156,6 +1162,7 @@ async fn test_receive_argument_by_immut_ref() -> Result<(), anyhow::Error> {
         build_config,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -1280,6 +1287,7 @@ async fn test_receive_argument_by_mut_ref() -> Result<(), anyhow::Error> {
         build_config,
         skip_dependency_verification: false,
         with_unpublished_dependencies: false,
+        verify_deps: true,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
     }
     .execute(context)
@@ -1406,6 +1414,7 @@ async fn test_package_publish_command_with_unpublished_dependency_succeeds(
         build_config,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies,
     }
     .execute(context)
@@ -1475,6 +1484,7 @@ async fn test_package_publish_command_with_unpublished_dependency_fails(
         build_config,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies,
     }
     .execute(context)
@@ -1518,6 +1528,7 @@ async fn test_package_publish_command_non_zero_unpublished_dep_fails() -> Result
         build_config,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies,
     }
     .execute(context)
@@ -1570,6 +1581,7 @@ async fn test_package_publish_command_failure_invalid() -> Result<(), anyhow::Er
         build_config,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies,
     }
     .execute(context)
@@ -1609,6 +1621,7 @@ async fn test_package_publish_nonexistent_dependency() -> Result<(), anyhow::Err
         build_config,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -1649,6 +1662,7 @@ async fn test_package_publish_test_flag() -> Result<(), anyhow::Error> {
         build_config,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -1701,6 +1715,7 @@ async fn test_package_upgrade_command() -> Result<(), anyhow::Error> {
         build_config,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -1772,6 +1787,7 @@ async fn test_package_upgrade_command() -> Result<(), anyhow::Error> {
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         verify_compatibility: true,
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -1837,6 +1853,7 @@ async fn test_package_management_on_upgrade_command() -> Result<(), anyhow::Erro
         build_config: build_config.clone(),
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -1891,6 +1908,7 @@ async fn test_package_management_on_upgrade_command() -> Result<(), anyhow::Erro
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         verify_compatibility: true,
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -1971,6 +1989,7 @@ async fn test_package_management_on_upgrade_command_conflict() -> Result<(), any
         build_config: build_config_publish.clone(),
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -2039,6 +2058,7 @@ async fn test_package_management_on_upgrade_command_conflict() -> Result<(), any
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
         verify_compatibility: true,
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
     }
     .execute(context)
@@ -3808,6 +3828,7 @@ async fn test_clever_errors() -> Result<(), anyhow::Error> {
         package_path: package_path.clone(),
         build_config,
         skip_dependency_verification: false,
+        verify_deps: true,
         with_unpublished_dependencies: false,
         opts: OptsWithGas::for_testing(Some(gas_obj_id), rgp * TEST_ONLY_GAS_UNIT_FOR_PUBLISH),
     }

diff --git a/crates/sui/tests/shell_tests.rs b/crates/sui/tests/shell_tests.rs
@@ -48,6 +48,7 @@ async fn test_shell_snapshot(path: &Path) -> datatest_stable::Result<()> {
             "PATH",
             format!("{}:{}", get_sui_bin_path(), std::env::var("PATH")?),
         )
+        .env("RUST_BACKTRACE", "0")
         .current_dir(sandbox)
         .arg(path.file_name().unwrap());
 

diff --git a/...ove_build_bytecode_with_address_resolution/move_build_bytecode_with_address_resolution.sh b/...ove_build_bytecode_with_address_resolution/move_build_bytecode_with_address_resolution.sh
@@ -2,7 +2,7 @@
 # SPDX-License-Identifier: Apache-2.0
 
 sui client --client.config $CONFIG \
-  publish simple \
+  publish simple --verify-deps \
   --json | jq '.effects.status'
 
 sui move --client.config $CONFIG \

diff --git a/...es/sui/tests/shell_tests/with_network/source_verification_deprecation/README.md b/...es/sui/tests/shell_tests/with_network/source_verification_deprecation/README.md
@@ -0,0 +1,2 @@
+This test suite checks that the deprecation warnings for dependency verification during publication and the
+associated flags `--skip-dependency-verification` and `--verify-deps` are working correctly.
diff --git a/crates/sui/tests/shell_tests/with_network/source_verification_deprecation/both_flags.sh b/crates/sui/tests/shell_tests/with_network/source_verification_deprecation/both_flags.sh
@@ -0,0 +1,10 @@
+# Copyright (c) Mysten Labs, Inc.
+# SPDX-License-Identifier: Apache-2.0
+
+# test that we get an error if we supply both `--skip-dependency-verification` and `--verify-deps`
+
+echo "=== publish ===" | tee /dev/stderr
+sui client --client.config $CONFIG publish example --skip-dependency-verification --verify-deps
+
+echo "=== upgrade ===" | tee /dev/stderr
+sui client --client.config $CONFIG upgrade example --upgrade-capability 0x1234 --skip-dependency-verification --verify-deps
diff --git a/...s/sui/tests/shell_tests/with_network/source_verification_deprecation/dependency/Move.toml b/...s/sui/tests/shell_tests/with_network/source_verification_deprecation/dependency/Move.toml
@@ -0,0 +1,10 @@
+[package]
+name = "dependency"
+edition = "2024.beta"
+
+[dependencies]
+# Sui = { local = "FRAMEWORK_DIR", override = true }
+
+[addresses]
+dependency = "0x0"
+
diff --git a/...ell_tests/with_network/source_verification_deprecation/dependency/sources/dependency.move b/...ell_tests/with_network/source_verification_deprecation/dependency/sources/dependency.move
@@ -0,0 +1,6 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+module dependency::dependency;
+
+public fun f(): u64 { 0 }
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
@@ -0,0 +1,10 @@
+[package]
+name = "example"
+edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move
+
+[dependencies]
+# Sui = { local = "FRAMEWORK_DIR" }
+dependency = { local = "../dependency" }
+
+[addresses]
+example = "0x0"
diff --git a/...sts/shell_tests/with_network/source_verification_deprecation/example/sources/example.move b/...sts/shell_tests/with_network/source_verification_deprecation/example/sources/example.move
@@ -0,0 +1,9 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+/// Module: example
+module example::example;
+
+use dependency::dependency::f;
+
+public fun g(): u64 { f() }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		This test suite checks that the deprecation warnings for dependency verification during publication and the
		associated flags `--skip-dependency-verification` and `--verify-deps` are working correctly.