[execution] Update to deps-only mode (#20102)

## Description 

Disallow new modules from being added to packages in deps-only mode. 

## Test plan 

Added new tests to check for both previous and new behavior.

---

## 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.

- [X] Protocol: Updated restrictions on deps-only packages to not allow
adding new modules to the package.
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:

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(())
     }