[execution] Update to deps-only mode

crates/sui-core/src/unit_tests/move_package_upgrade_tests.rs

@@ -15,7 +15,12 @@ use sui_types::{
     MOVE_STDLIB_PACKAGE_ID, SUI_FRAMEWORK_PACKAGE_ID,
 };
 
-use std::{collections::BTreeSet, path::PathBuf, str::FromStr, sync::Arc};
+use std::{
+    collections::BTreeSet,
+    path::{Path, PathBuf},
+    str::FromStr,
+    sync::Arc,
+};
 use sui_types::effects::{TransactionEffects, TransactionEffectsAPI};
 use sui_types::error::{SuiError, UserInputError};
 use sui_types::execution_config_utils::to_binary_config;
@@ -48,11 +53,62 @@ macro_rules! move_call {
     }
 }
 
+enum FileOverlay<'a> {
+    Remove(&'a str),
+    Add {
+        file_name: &'a str,
+        contents: &'a str,
+    },
+}
+
+fn build_upgrade_test_modules_with_overlay(
+    base_pkg: &str,
+    overlay: FileOverlay<'_>,
+) -> (Vec<u8>, Vec<Vec<u8>>) {
+    // Root temp dirs under `move_upgrade` directory so that dependency paths remain correct.
+    let mut tmp_dir_root_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+    tmp_dir_root_path.extend(["src", "unit_tests", "data", "move_upgrade"]);
+
+    let tmp_dir = tempfile::TempDir::new_in(tmp_dir_root_path).unwrap();
+    let tmp_dir_path = tmp_dir.path();
+
+    let mut copy_options = fs_extra::dir::CopyOptions::new();
+    copy_options.copy_inside = true;
+    copy_options.content_only = true;
+    let source_dir = pkg_path_of(base_pkg);
+    fs_extra::dir::copy(source_dir, tmp_dir_path, &copy_options).unwrap();
+
+    match overlay {
+        FileOverlay::Remove(file_name) => {
+            let file_path = tmp_dir_path.join(format!("sources/{}", file_name));
+            std::fs::remove_file(file_path).unwrap();
+        }
+        FileOverlay::Add {
+            file_name,
+            contents,
+        } => {
+            let new_file_path = tmp_dir_path.join(format!("sources/{}", file_name));
+            std::fs::write(new_file_path, contents).unwrap();
+        }
+    }
+
+    build_pkg_at_path(tmp_dir_path)
+}
+
 fn build_upgrade_test_modules(test_dir: &str) -> (Vec<u8>, Vec<Vec<u8>>) {
+    let path = pkg_path_of(test_dir);
+    build_pkg_at_path(&path)
+}
+
+fn pkg_path_of(pkg_name: &str) -> PathBuf {
     let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
-    path.extend(["src", "unit_tests", "data", "move_upgrade", test_dir]);
+    path.extend(["src", "unit_tests", "data", "move_upgrade", pkg_name]);
+    path
+}
+
+fn build_pkg_at_path(path: &Path) -> (Vec<u8>, Vec<Vec<u8>>) {
     let with_unpublished_deps = false;
-    let package = BuildConfig::new_for_testing().build(&path).unwrap();
+    let package = BuildConfig::new_for_testing().build(path).unwrap();
     (
         package.get_package_digest(with_unpublished_deps).to_vec(),
         package.get_package_bytes(with_unpublished_deps),
@@ -457,6 +513,116 @@ async fn test_upgrade_package_compatible_in_dep_only_mode() {
     );
 }
 
+#[tokio::test]
+async fn test_upgrade_package_add_new_module_in_dep_only_mode_pre_v68() {
+    // Allow new modules in deps-only mode for this test.
+    let _guard = ProtocolConfig::apply_overrides_for_testing(|_, mut config| {
+        config.set_disallow_new_modules_in_deps_only_packages_for_testing(false);
+        config
+    });
+
+    let mut runner = UpgradeStateRunner::new("move_upgrade/base").await;
+    let base_pkg = "dep_only_upgrade";
+    assert_valid_dep_only_upgrade(&mut runner, base_pkg).await;
+    let (digest, modules) = build_upgrade_test_modules_with_overlay(
+        base_pkg,
+        FileOverlay::Add {
+            file_name: "new_module.move",
+            contents: "module base_addr::new_module;",
+        },
+    );
+    let effects = runner
+        .upgrade(
+            UpgradePolicy::DEP_ONLY,
+            digest,
+            modules,
+            vec![SUI_FRAMEWORK_PACKAGE_ID, MOVE_STDLIB_PACKAGE_ID],
+        )
+        .await;
+
+    assert!(effects.status().is_ok(), "{:#?}", effects.status());
+}
+
+#[tokio::test]
+async fn test_upgrade_package_invalid_dep_only_upgrade_pre_v68() {
+    let _guard = ProtocolConfig::apply_overrides_for_testing(|_, mut config| {
+        config.set_disallow_new_modules_in_deps_only_packages_for_testing(false);
+        config
+    });
+
+    let mut runner = UpgradeStateRunner::new("move_upgrade/base").await;
+    let base_pkg = "dep_only_upgrade";
+    assert_valid_dep_only_upgrade(&mut runner, base_pkg).await;
+    let overlays = [
+        FileOverlay::Add {
+            file_name: "new_friend_module.move",
+            contents: r#"
+module base_addr::new_friend_module; 
+public fun friend_call(): u64 { base_addr::base::friend_fun(1) }
+        "#,
+        },
+        FileOverlay::Remove("friend_module.move"),
+    ];
+    for overlay in overlays {
+        let (digest, modules) = build_upgrade_test_modules_with_overlay(base_pkg, overlay);
+        let effects = runner
+            .upgrade(
+                UpgradePolicy::DEP_ONLY,
+                digest,
+                modules,
+                vec![SUI_FRAMEWORK_PACKAGE_ID, MOVE_STDLIB_PACKAGE_ID],
+            )
+            .await;
+
+        assert_eq!(
+            effects.into_status().unwrap_err().0,
+            ExecutionFailureStatus::PackageUpgradeError {
+                upgrade_error: PackageUpgradeError::IncompatibleUpgrade
+            },
+        );
+    }
+}
+
+#[tokio::test]
+async fn test_invalid_dep_only_upgrades() {
+    let mut runner = UpgradeStateRunner::new("move_upgrade/base").await;
+    let base_pkg = "dep_only_upgrade";
+    assert_valid_dep_only_upgrade(&mut runner, base_pkg).await;
+    let overlays = [
+        FileOverlay::Add {
+            file_name: "new_module.move",
+            contents: "module base_addr::new_module;",
+        },
+        FileOverlay::Add {
+            file_name: "new_friend_module.move",
+            contents: r#"
+module base_addr::new_friend_module; 
+public fun friend_call(): u64 { base_addr::base::friend_fun(1) }
+        "#,
+        },
+        FileOverlay::Remove("friend_module.move"),
+    ];
+
+    for overlay in overlays {
+        let (digest, modules) = build_upgrade_test_modules_with_overlay(base_pkg, overlay);
+        let effects = runner
+            .upgrade(
+                UpgradePolicy::DEP_ONLY,
+                digest,
+                modules,
+                vec![SUI_FRAMEWORK_PACKAGE_ID, MOVE_STDLIB_PACKAGE_ID],
+            )
+            .await;
+
+        assert_eq!(
+            effects.into_status().unwrap_err().0,
+            ExecutionFailureStatus::PackageUpgradeError {
+                upgrade_error: PackageUpgradeError::IncompatibleUpgrade
+            },
+        );
+    }
+}
+
 #[tokio::test]
 async fn test_upgrade_package_compatible_in_additive_mode() {
     let mut runner = UpgradeStateRunner::new("move_upgrade/base").await;
@@ -572,18 +738,7 @@ async fn test_upgrade_package_additive_dep_only_mode() {
 #[tokio::test]
 async fn test_upgrade_package_dep_only_mode() {
     let mut runner = UpgradeStateRunner::new("move_upgrade/base").await;
-
-    let (digest, modules) = build_upgrade_test_modules("dep_only_upgrade");
-    let effects = runner
-        .upgrade(
-            UpgradePolicy::DEP_ONLY,
-            digest,
-            modules,
-            vec![SUI_FRAMEWORK_PACKAGE_ID, MOVE_STDLIB_PACKAGE_ID],
-        )
-        .await;
-
-    assert!(effects.status().is_ok(), "{:#?}", effects.status());
+    assert_valid_dep_only_upgrade(&mut runner, "dep_only_upgrade").await;
 }
 
 #[tokio::test]
@@ -1432,3 +1587,17 @@ async fn test_upgrade_more_than_max_packages_error() {
         }
     );
 }
+
+async fn assert_valid_dep_only_upgrade(runner: &mut UpgradeStateRunner, package_name: &str) {
+    let (digest, modules) = build_upgrade_test_modules(package_name);
+    let effects = runner
+        .upgrade(
+            UpgradePolicy::DEP_ONLY,
+            digest,
+            modules,
+            vec![SUI_FRAMEWORK_PACKAGE_ID, MOVE_STDLIB_PACKAGE_ID],
+        )
+        .await;
+
+    assert!(effects.status().is_ok(), "{:#?}", effects.status());
+}

crates/sui-open-rpc/spec/openrpc.json

@@ -1310,6 +1310,7 @@
                 "disable_invariant_violation_check_in_swap_loc": false,
                 "disallow_adding_abilities_on_upgrade": false,
                 "disallow_change_struct_type_params_on_upgrade": false,
+                "disallow_new_modules_in_deps_only_packages": false,
                 "enable_coin_deny_list": false,
                 "enable_coin_deny_list_v2": false,
                 "enable_effects_v2": false,

crates/sui-protocol-config/src/lib.rs

@@ -194,6 +194,7 @@ const MAX_PROTOCOL_VERSION: u64 = 68;
 //             Update to Move stdlib.
 //             Enable gas based congestion control with overage.
 //             Further reduce minimum number of random beacon shares.
+//             Disallow adding new modules in `deps-only` packages.
 
 #[derive(Copy, Clone, Debug, Hash, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct ProtocolVersion(u64);
@@ -560,6 +561,9 @@ struct FeatureFlags {
     // Enable uncompressed group elements in BLS123-81 G1
     #[serde(skip_serializing_if = "is_false")]
     uncompressed_g1_group_elements: bool,
+
+    #[serde(skip_serializing_if = "is_false")]
+    disallow_new_modules_in_deps_only_packages: bool,
 }
 
 fn is_false(b: &bool) -> bool {
@@ -1659,6 +1663,11 @@ impl ProtocolConfig {
     pub fn uncompressed_g1_group_elements(&self) -> bool {
         self.feature_flags.uncompressed_g1_group_elements
     }
+
+    pub fn disallow_new_modules_in_deps_only_packages(&self) -> bool {
+        self.feature_flags
+            .disallow_new_modules_in_deps_only_packages
+    }
 }
 
 #[cfg(not(msim))]
@@ -2920,6 +2929,8 @@ impl ProtocolConfig {
 
                     // Further reduce minimum number of random beacon shares.
                     cfg.random_beacon_reduction_lower_bound = Some(500);
+
+                    cfg.feature_flags.disallow_new_modules_in_deps_only_packages = true;
                 }
                 // Use this template when making changes:
                 //
@@ -3086,6 +3097,11 @@ impl ProtocolConfig {
     pub fn set_gc_depth_for_testing(&mut self, val: u32) {
         self.consensus_gc_depth = Some(val);
     }
+
+    pub fn set_disallow_new_modules_in_deps_only_packages_for_testing(&mut self, val: bool) {
+        self.feature_flags
+            .disallow_new_modules_in_deps_only_packages = val;
+    }
 }
 
 type OverrideFn = dyn Fn(ProtocolVersion, ProtocolConfig) -> ProtocolConfig + Send;

crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Mainnet_version_68.snap

@@ -64,6 +64,7 @@ feature_flags:
   consensus_round_prober: true
   validate_identifier_inputs: true
   relocate_event_module: true
+  disallow_new_modules_in_deps_only_packages: true
 max_tx_size_bytes: 131072
 max_input_objects: 2048
 max_size_written_objects: 5000000

crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_68.snap

@@ -64,6 +64,7 @@ feature_flags:
   consensus_round_prober: true
   validate_identifier_inputs: true
   relocate_event_module: true
+  disallow_new_modules_in_deps_only_packages: true
 max_tx_size_bytes: 131072
 max_input_objects: 2048
 max_size_written_objects: 5000000

crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_68.snap

@@ -71,6 +71,7 @@ feature_flags:
   mysticeti_fastpath: true
   relocate_event_module: true
   uncompressed_g1_group_elements: true
+  disallow_new_modules_in_deps_only_packages: true
 max_tx_size_bytes: 131072
 max_input_objects: 2048
 max_size_written_objects: 5000000

sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs

@@ -640,10 +640,10 @@ mod checked {
         )])
     }
 
-    fn check_compatibility<'a>(
+    fn check_compatibility(
         context: &ExecutionContext,
         existing_package: &MovePackage,
-        upgrading_modules: impl IntoIterator<Item = &'a CompiledModule>,
+        upgrading_modules: &[CompiledModule],
         policy: u8,
     ) -> Result<(), ExecutionError> {
         // Make sure this is a known upgrade policy.
@@ -660,7 +660,26 @@ mod checked {
             invariant_violation!("Tried to normalize modules in existing package but failed")
         };
 
-        let mut new_normalized = normalize_deserialized_modules(upgrading_modules.into_iter());
+        let existing_modules_len = current_normalized.len();
+        let upgrading_modules_len = upgrading_modules.len();
+        let disallow_new_modules = context
+            .protocol_config
+            .disallow_new_modules_in_deps_only_packages()
+            && policy as u8 == UpgradePolicy::DEP_ONLY;
+
+        if disallow_new_modules && existing_modules_len != upgrading_modules_len {
+            return Err(ExecutionError::new_with_source(
+                ExecutionErrorKind::PackageUpgradeError {
+                    upgrade_error: PackageUpgradeError::IncompatibleUpgrade,
+                },
+                format!(
+                    "Existing package has {existing_modules_len} modules, but new package has \
+                     {upgrading_modules_len}. Adding or removing a module to a deps only package is not allowed."
+                ),
+            ));
+        }
+
+        let mut new_normalized = normalize_deserialized_modules(upgrading_modules.iter());
         for (name, cur_module) in current_normalized {
             let Some(new_module) = new_normalized.remove(&name) else {
                 return Err(ExecutionError::new_with_source(
@@ -674,6 +693,9 @@ mod checked {
             check_module_compatibility(&policy, &cur_module, &new_module)?;
         }
 
+        // If we disallow new modules double check that there are no modules left in `new_normalized`.
+        debug_assert!(!disallow_new_modules || new_normalized.is_empty());
+
         Ok(())
     }