From 1f16ff8e20bd1d4265968972ca468528ebf683cb Mon Sep 17 00:00:00 2001 From: Timothy Zakian Date: Wed, 30 Oct 2024 14:29:57 -0700 Subject: [PATCH] [execution] Update to deps-only mode --- .../Move.toml | 10 ++ .../sources/base.move | 20 +++ .../sources/friend_module.move | 18 +++ .../sources/new_friend_module.move | 6 + .../dep_only_upgrade_new_module/Move.toml | 10 ++ .../sources/base.move | 20 +++ .../sources/friend_module.move | 18 +++ .../sources/new_module.move | 4 + .../dep_only_upgrade_remove_module/Move.toml | 10 ++ .../sources/base.move | 20 +++ .../unit_tests/move_package_upgrade_tests.rs | 116 ++++++++++++++++++ crates/sui-open-rpc/spec/openrpc.json | 1 + crates/sui-protocol-config/src/lib.rs | 16 +++ ...ocol_config__test__Mainnet_version_68.snap | 1 + ...ocol_config__test__Testnet_version_68.snap | 1 + ...sui_protocol_config__test__version_68.snap | 1 + .../programmable_transactions/execution.rs | 21 +++- 17 files changed, 292 insertions(+), 1 deletion(-) create mode 100644 crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/Move.toml create mode 100644 crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/sources/base.move create mode 100644 crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/sources/friend_module.move create mode 100644 crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/sources/new_friend_module.move create mode 100644 crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/Move.toml create mode 100644 crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/sources/base.move create mode 100644 crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/sources/friend_module.move create mode 100644 crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/sources/new_module.move create mode 100644 crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_remove_module/Move.toml create mode 100644 crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_remove_module/sources/base.move diff --git a/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/Move.toml b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/Move.toml new file mode 100644 index 0000000000000..f011a1a17e686 --- /dev/null +++ b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/Move.toml @@ -0,0 +1,10 @@ +[package] +name = "package_upgrade_base" +version = "0.0.1" +edition = "2024.beta" + +[addresses] +base_addr = "0x0" + +[dependencies] +Sui = { local = "../../../../../../sui-framework/packages/sui-framework" } diff --git a/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/sources/base.move b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/sources/base.move new file mode 100644 index 0000000000000..72f006e60cff4 --- /dev/null +++ b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/sources/base.move @@ -0,0 +1,20 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +module base_addr::base { + + public struct A { + f1: bool, + f2: T + } + + public fun return_0(): u64 { abort 42 } + + public fun plus_1(x: u64): u64 { x + 1 } + + public(package) fun friend_fun(x: u64): u64 { x } + + fun non_public_fun(y: bool): u64 { if (y) 0 else 1 } + + entry fun entry_fun() { } +} diff --git a/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/sources/friend_module.move b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/sources/friend_module.move new file mode 100644 index 0000000000000..474ad93c69325 --- /dev/null +++ b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/sources/friend_module.move @@ -0,0 +1,18 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +module base_addr::friend_module { + + public struct A { + field1: u64, + field2: T + } + + public fun friend_call(): u64 { base_addr::base::friend_fun(1) } + + public fun return_0(): u64 { 0 } + + public fun plus_1(x: u64): u64 { x + 1 } + + fun non_public_fun(y: bool): u64 { if (y) 0 else 1 } +} diff --git a/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/sources/new_friend_module.move b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/sources/new_friend_module.move new file mode 100644 index 0000000000000..04d2694736316 --- /dev/null +++ b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_friend_module/sources/new_friend_module.move @@ -0,0 +1,6 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +module base_addr::new_friend_module; + +public fun friend_call(): u64 { base_addr::base::friend_fun(1) } diff --git a/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/Move.toml b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/Move.toml new file mode 100644 index 0000000000000..f011a1a17e686 --- /dev/null +++ b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/Move.toml @@ -0,0 +1,10 @@ +[package] +name = "package_upgrade_base" +version = "0.0.1" +edition = "2024.beta" + +[addresses] +base_addr = "0x0" + +[dependencies] +Sui = { local = "../../../../../../sui-framework/packages/sui-framework" } diff --git a/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/sources/base.move b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/sources/base.move new file mode 100644 index 0000000000000..72f006e60cff4 --- /dev/null +++ b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/sources/base.move @@ -0,0 +1,20 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +module base_addr::base { + + public struct A { + f1: bool, + f2: T + } + + public fun return_0(): u64 { abort 42 } + + public fun plus_1(x: u64): u64 { x + 1 } + + public(package) fun friend_fun(x: u64): u64 { x } + + fun non_public_fun(y: bool): u64 { if (y) 0 else 1 } + + entry fun entry_fun() { } +} diff --git a/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/sources/friend_module.move b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/sources/friend_module.move new file mode 100644 index 0000000000000..474ad93c69325 --- /dev/null +++ b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/sources/friend_module.move @@ -0,0 +1,18 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +module base_addr::friend_module { + + public struct A { + field1: u64, + field2: T + } + + public fun friend_call(): u64 { base_addr::base::friend_fun(1) } + + public fun return_0(): u64 { 0 } + + public fun plus_1(x: u64): u64 { x + 1 } + + fun non_public_fun(y: bool): u64 { if (y) 0 else 1 } +} diff --git a/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/sources/new_module.move b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/sources/new_module.move new file mode 100644 index 0000000000000..2a96fd527cd2a --- /dev/null +++ b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_new_module/sources/new_module.move @@ -0,0 +1,4 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +module base_addr::new_module; diff --git a/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_remove_module/Move.toml b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_remove_module/Move.toml new file mode 100644 index 0000000000000..f011a1a17e686 --- /dev/null +++ b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_remove_module/Move.toml @@ -0,0 +1,10 @@ +[package] +name = "package_upgrade_base" +version = "0.0.1" +edition = "2024.beta" + +[addresses] +base_addr = "0x0" + +[dependencies] +Sui = { local = "../../../../../../sui-framework/packages/sui-framework" } diff --git a/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_remove_module/sources/base.move b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_remove_module/sources/base.move new file mode 100644 index 0000000000000..72f006e60cff4 --- /dev/null +++ b/crates/sui-core/src/unit_tests/data/move_upgrade/dep_only_upgrade_remove_module/sources/base.move @@ -0,0 +1,20 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +module base_addr::base { + + public struct A { + f1: bool, + f2: T + } + + public fun return_0(): u64 { abort 42 } + + public fun plus_1(x: u64): u64 { x + 1 } + + public(package) fun friend_fun(x: u64): u64 { x } + + fun non_public_fun(y: bool): u64 { if (y) 0 else 1 } + + entry fun entry_fun() { } +} diff --git a/crates/sui-core/src/unit_tests/move_package_upgrade_tests.rs b/crates/sui-core/src/unit_tests/move_package_upgrade_tests.rs index eed9dcf2af4be..4efa2d45e44b2 100644 --- a/crates/sui-core/src/unit_tests/move_package_upgrade_tests.rs +++ b/crates/sui-core/src/unit_tests/move_package_upgrade_tests.rs @@ -457,6 +457,122 @@ 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 (digest, modules) = build_upgrade_test_modules("dep_only_upgrade_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_add_new_module_in_dep_only_mode() { + let mut runner = UpgradeStateRunner::new("move_upgrade/base").await; + + let (digest, modules) = build_upgrade_test_modules("dep_only_upgrade_new_module"); + 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_add_new_friend_module_in_dep_only_mode_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 (digest, modules) = build_upgrade_test_modules("dep_only_upgrade_new_friend_module"); + 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_add_new_friend_module_in_dep_only_mode() { + let mut runner = UpgradeStateRunner::new("move_upgrade/base").await; + + let (digest, modules) = build_upgrade_test_modules("dep_only_upgrade_new_friend_module"); + 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_remove_module_in_dep_only_mode() { + let mut runner = UpgradeStateRunner::new("move_upgrade/base").await; + + let (digest, modules) = build_upgrade_test_modules("dep_only_upgrade_remove_module"); + 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; diff --git a/crates/sui-open-rpc/spec/openrpc.json b/crates/sui-open-rpc/spec/openrpc.json index d504bad18c95c..de8467490bdab 100644 --- a/crates/sui-open-rpc/spec/openrpc.json +++ b/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, diff --git a/crates/sui-protocol-config/src/lib.rs b/crates/sui-protocol-config/src/lib.rs index 08c5991f28371..caccb8d70d662 100644 --- a/crates/sui-protocol-config/src/lib.rs +++ b/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; diff --git a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Mainnet_version_68.snap b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Mainnet_version_68.snap index e05651d46bef8..492aebef1a352 100644 --- a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Mainnet_version_68.snap +++ b/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 diff --git a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_68.snap b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_68.snap index e05651d46bef8..492aebef1a352 100644 --- a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__Testnet_version_68.snap +++ b/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 diff --git a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_68.snap b/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_68.snap index 613b09985531d..68e55955524b4 100644 --- a/crates/sui-protocol-config/src/snapshots/sui_protocol_config__test__version_68.snap +++ b/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 diff --git a/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs b/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs index 12f144fbb04cf..7f8d9b9f67b6e 100644 --- a/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs +++ b/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs @@ -643,7 +643,7 @@ mod checked { fn check_compatibility<'a>( context: &ExecutionContext, existing_package: &MovePackage, - upgrading_modules: impl IntoIterator, + upgrading_modules: &[CompiledModule], policy: u8, ) -> Result<(), ExecutionError> { // Make sure this is a known upgrade policy. @@ -660,6 +660,25 @@ mod checked { invariant_violation!("Tried to normalize modules in existing package but failed") }; + let existing_modules_len = current_normalized.len(); + let upgrading_modules_len = upgrading_modules.len(); + if context + .protocol_config + .disallow_new_modules_in_deps_only_packages() + && policy as u8 == UpgradePolicy::DEP_ONLY + && 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.into_iter()); for (name, cur_module) in current_normalized { let Some(new_module) = new_normalized.remove(&name) else {