diff --git a/Cargo.lock b/Cargo.lock index 854cd8621bc..c055e84ee30 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6359,6 +6359,7 @@ dependencies = [ "fastcrypto", "fastcrypto-tbls", "fastcrypto-zkp", + "fs_extra", "futures", "im", "indexmap 2.5.0", diff --git a/crates/iota-core/Cargo.toml b/crates/iota-core/Cargo.toml index ecc1e8f6463..95caa59d817 100644 --- a/crates/iota-core/Cargo.toml +++ b/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 diff --git a/crates/iota-core/src/unit_tests/move_package_upgrade_tests.rs b/crates/iota-core/src/unit_tests/move_package_upgrade_tests.rs index e61dce2f049..b414ef7d7c4 100644 --- a/crates/iota-core/src/unit_tests/move_package_upgrade_tests.rs +++ b/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, Vec>) { + // 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, ©_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, Vec>) { + 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, Vec>) { 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()); +} diff --git a/crates/iota-open-rpc/spec/openrpc.json b/crates/iota-open-rpc/spec/openrpc.json index df0b351e069..8e45efe03ed 100644 --- a/crates/iota-open-rpc/spec/openrpc.json +++ b/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, diff --git a/crates/iota-protocol-config/src/lib.rs b/crates/iota-protocol-config/src/lib.rs index 780816d42f5..c757ead842c 100644 --- a/crates/iota-protocol-config/src/lib.rs +++ b/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; diff --git a/crates/iota-protocol-config/src/snapshots/iota_protocol_config__test__Mainnet_version_5.snap b/crates/iota-protocol-config/src/snapshots/iota_protocol_config__test__Mainnet_version_5.snap index f1ae2adeba2..2afa1e0130d 100644 --- a/crates/iota-protocol-config/src/snapshots/iota_protocol_config__test__Mainnet_version_5.snap +++ b/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 diff --git a/crates/iota-protocol-config/src/snapshots/iota_protocol_config__test__Testnet_version_5.snap b/crates/iota-protocol-config/src/snapshots/iota_protocol_config__test__Testnet_version_5.snap index 2df256f3cf8..f281ad70470 100644 --- a/crates/iota-protocol-config/src/snapshots/iota_protocol_config__test__Testnet_version_5.snap +++ b/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 diff --git a/crates/iota-protocol-config/src/snapshots/iota_protocol_config__test__version_5.snap b/crates/iota-protocol-config/src/snapshots/iota_protocol_config__test__version_5.snap index 3816536d098..16a9f1ba738 100644 --- a/crates/iota-protocol-config/src/snapshots/iota_protocol_config__test__version_5.snap +++ b/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 diff --git a/iota-execution/latest/iota-adapter/src/programmable_transactions/execution.rs b/iota-execution/latest/iota-adapter/src/programmable_transactions/execution.rs index 22cfe89b8b3..80d948e692d 100644 --- a/iota-execution/latest/iota-adapter/src/programmable_transactions/execution.rs +++ b/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, + 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(()) }