feat: disallow new modules from being added to packages in deps-only mode

crates/iota-core/Cargo.toml

@@ -98,6 +98,7 @@ typed-store-derive.workspace = true
 clap.workspace = true
 criterion.workspace = true
 expect-test.workspace = true
+fs_extra.workspace = true
 more-asserts.workspace = true
 num-bigint.workspace = true
 pretty_assertions.workspace = true

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

@@ -2,7 +2,12 @@
 // Modifications Copyright (c) 2024 IOTA Stiftung
 // SPDX-License-Identifier: Apache-2.0
 
-use std::{collections::BTreeSet, path::PathBuf, str::FromStr, sync::Arc};
+use std::{
+    collections::BTreeSet,
+    path::{Path, PathBuf},
+    str::FromStr,
+    sync::Arc,
+};
 
 use iota_move_build::BuildConfig;
 use iota_protocol_config::ProtocolConfig;
@@ -48,11 +53,63 @@ 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),
@@ -462,6 +519,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_v5() {
+    // 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![IOTA_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_v5() {
+    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![IOTA_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![IOTA_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;
@@ -578,17 +745,7 @@ async fn test_upgrade_package_additive_dep_only_mode() {
 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![IOTA_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]
@@ -1445,3 +1602,16 @@ 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![IOTA_FRAMEWORK_PACKAGE_ID, MOVE_STDLIB_PACKAGE_ID],
+        )
+        .await;
+    assert!(effects.status().is_ok(), "{:#?}", effects.status());
+}

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

@@ -1306,6 +1306,7 @@
                 "accept_zklogin_in_multisig": false,
                 "bridge": false,
                 "disable_invariant_violation_check_in_swap_loc": true,
+                "disallow_new_modules_in_deps_only_packages": false,
                 "enable_group_ops_native_function_msm": true,
                 "enable_jwk_consensus_updates": false,
                 "enable_poseidon": true,

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

@@ -28,7 +28,7 @@ pub const MAX_PROTOCOL_VERSION: u64 = 5;
 // Add `Clock` based unlock to `Timelock` objects.
 // Version 4: Introduce the `max_type_to_layout_nodes` config that sets the
 // maximal nodes which are allowed when converting to a type layout.
-// Version 5: TODO.
+// Version 5: Disallow adding new modules in `deps-only` packages.
 
 #[derive(Copy, Clone, Debug, Hash, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct ProtocolVersion(u64);
@@ -187,6 +187,10 @@ struct FeatureFlags {
     // Makes the event's sending module version-aware.
     #[serde(skip_serializing_if = "is_false")]
     relocate_event_module: bool,
+
+    // Disallow adding new modules in `deps-only` packages.
+    #[serde(skip_serializing_if = "is_false")]
+    disallow_new_modules_in_deps_only_packages: bool,
 }
 
 fn is_true(b: &bool) -> bool {
@@ -1075,6 +1079,11 @@ impl ProtocolConfig {
     pub fn relocate_event_module(&self) -> bool {
         self.feature_flags.relocate_event_module
     }
+
+    pub fn disallow_new_modules_in_deps_only_packages(&self) -> bool {
+        self.feature_flags
+            .disallow_new_modules_in_deps_only_packages
+    }
 }
 
 #[cfg(not(msim))]
@@ -1668,7 +1677,9 @@ impl ProtocolConfig {
                 4 => {
                     cfg.max_type_to_layout_nodes = Some(512);
                 }
-                5 => {}
+                5 => {
+                    cfg.feature_flags.disallow_new_modules_in_deps_only_packages = true;
+                }
                 // Use this template when making changes:
                 //
                 //     // modify an existing constant.
@@ -1789,6 +1800,11 @@ impl ProtocolConfig {
     pub fn set_passkey_auth_for_testing(&mut self, val: bool) {
         self.feature_flags.passkey_auth = 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 + Sync;

crates/iota-protocol-config/src/snapshots/iota_protocol_config__test__Mainnet_version_5.snap

@@ -8,6 +8,7 @@ feature_flags:
   per_object_congestion_control_mode: TotalTxCount
   zklogin_max_epoch_upper_bound_delta: 30
   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/iota-protocol-config/src/snapshots/iota_protocol_config__test__Testnet_version_5.snap

@@ -8,6 +8,7 @@ feature_flags:
   per_object_congestion_control_mode: TotalTxCount
   zklogin_max_epoch_upper_bound_delta: 30
   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/iota-protocol-config/src/snapshots/iota_protocol_config__test__version_5.snap

@@ -12,6 +12,7 @@ feature_flags:
   enable_vdf: true
   passkey_auth: 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

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

@@ -665,10 +665,10 @@ mod checked {
     /// function first validates the upgrade policy, then normalizes the
     /// existing and new modules to compare them. For each module, it verifies
     /// compatibility according to the policy.
-    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.
@@ -685,7 +685,25 @@ 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(
@@ -699,6 +717,10 @@ 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(())
     }