[Framework] Add dependencies check for code object freezing (#14064) (#…

…14079)

* Refactor object deployment CLI commands

* Refactor object deployment CLI commands

---------

Co-authored-by: Kevin <[email protected]>
Co-authored-by: JohnChangUK <[email protected]>

aptos-move/e2e-move-tests/src/tests/object_code_deployment.data/pack_immutable_deps/Move.toml

@@ -0,0 +1,7 @@
+[package]
+name = "test_immutable_deps"
+version = "0.0.0"
+upgrade_policy = "compatible"
+
+[dependencies]
+test_package = { local = "../pack_initial_immutable" }

aptos-move/e2e-move-tests/src/tests/object_code_deployment.data/pack_immutable_deps/sources/test_immutable_deps.move

@@ -0,0 +1,10 @@
+module object_immutable_deps::test_immutable_deps {
+
+    struct State has key {
+        value: u64
+    }
+
+    public entry fun hello(s: &signer, value: u64) {
+        move_to(s, State { value })
+    }
+}

aptos-move/e2e-move-tests/src/tests/object_code_deployment.data/pack_mutable_deps/Move.toml

@@ -0,0 +1,8 @@
+[package]
+name = "test_mutable_deps"
+version = "0.0.0"
+upgrade_policy = "compatible"
+
+[dependencies]
+AptosFramework = { local = "../../../../../framework/aptos-framework" }
+test_package = { local = "../pack_upgrade_compat" }

aptos-move/e2e-move-tests/src/tests/object_code_deployment.data/pack_mutable_deps/sources/test_mutable_deps.move

@@ -0,0 +1,10 @@
+module object_mutable_deps::test_mutable_deps {
+
+    struct State has key {
+        value: u64
+    }
+
+    public entry fun hello(s: &signer, value: u64) {
+        move_to(s, State { value })
+    }
+}

aptos-move/e2e-move-tests/src/tests/object_code_deployment.rs

@@ -74,7 +74,16 @@ impl TestContext {
         options
             .named_addresses
             .insert(MODULE_ADDRESS_NAME.to_string(), self.object_address);
+        self.execute_object_code_action_with_options(account, path, action, options)
+    }
 
+    fn execute_object_code_action_with_options(
+        &mut self,
+        account: &Account,
+        path: &str,
+        action: ObjectCodeAction,
+        options: BuildOptions,
+    ) -> TransactionStatus {
         match action {
             ObjectCodeAction::Deploy => self.harness.object_code_deployment_package(
                 account,
@@ -118,10 +127,13 @@ impl TestContext {
 }
 
 const MODULE_ADDRESS_NAME: &str = "object";
+const MUT_DEPS_MODULE_ADDRESS_NAME: &str = "object_mutable_deps";
+const IMMUT_DEPS_MODULE_ADDRESS_NAME: &str = "object_immutable_deps";
 const PACKAGE_REGISTRY_ACCESS_PATH: &str = "0x1::code::PackageRegistry";
 const EOBJECT_CODE_DEPLOYMENT_NOT_SUPPORTED: &str = "EOBJECT_CODE_DEPLOYMENT_NOT_SUPPORTED";
 const ENOT_CODE_OBJECT_OWNER: &str = "ENOT_CODE_OBJECT_OWNER";
 const ENOT_PACKAGE_OWNER: &str = "ENOT_PACKAGE_OWNER";
+const EDEP_WEAKER_POLICY: &str = "EDEP_WEAKER_POLICY";
 
 /// Tests the `publish` object code deployment function with feature flags enabled/disabled.
 /// Deployment should only happen when feature is enabled.
@@ -362,6 +374,75 @@ fn freeze_code_object_fail_when_not_owner() {
     context.assert_feature_flag_error(status, ENOT_PACKAGE_OWNER);
 }
 
+#[test]
+fn freeze_code_object_fail_when_having_mutable_dependency() {
+    let mut context = TestContext::new(None, None);
+    let acc = context.account.clone();
+
+    assert_success!(context.execute_object_code_action(
+        &acc,
+        "object_code_deployment.data/pack_upgrade_compat",
+        ObjectCodeAction::Deploy,
+    ));
+    let mut options = BuildOptions::default();
+    options
+        .named_addresses
+        .insert(MODULE_ADDRESS_NAME.to_string(), context.object_address);
+    let sequence_number = context.harness.sequence_number(acc.address());
+    context.object_address =
+        create_object_code_deployment_address(*acc.address(), sequence_number + 1);
+    options.named_addresses.insert(
+        MUT_DEPS_MODULE_ADDRESS_NAME.to_string(),
+        context.object_address,
+    );
+    assert_success!(context.execute_object_code_action_with_options(
+        &acc,
+        "object_code_deployment.data/pack_mutable_deps",
+        ObjectCodeAction::Deploy,
+        options
+    ));
+
+    // Freezing a package with upgradeable dependencies is not allowed.
+    let status = context.execute_object_code_action(&acc, "", ObjectCodeAction::Freeze);
+    context.assert_feature_flag_error(status, EDEP_WEAKER_POLICY);
+}
+
+#[test]
+fn freeze_code_object_succeeds_when_all_dependencies_immutable() {
+    let mut context = TestContext::new(None, None);
+    let acc = context.account.clone();
+
+    // Deploy immutable dependency package
+    assert_success!(context.execute_object_code_action(
+        &acc,
+        "object_code_deployment.data/pack_initial_immutable",
+        ObjectCodeAction::Deploy,
+    ));
+    let mut options = BuildOptions::default();
+    options
+        .named_addresses
+        .insert(MODULE_ADDRESS_NAME.to_string(), context.object_address);
+    let sequence_number = context.harness.sequence_number(acc.address());
+    context.object_address =
+        create_object_code_deployment_address(*acc.address(), sequence_number + 1);
+    options.named_addresses.insert(
+        IMMUT_DEPS_MODULE_ADDRESS_NAME.to_string(),
+        context.object_address,
+    );
+
+    // Deploy mutable package with immutable dependency
+    assert_success!(context.execute_object_code_action_with_options(
+        &acc,
+        "object_code_deployment.data/pack_immutable_deps",
+        ObjectCodeAction::Deploy,
+        options
+    ));
+
+    // Attempt to freeze the initial package
+    let status = context.execute_object_code_action(&acc, "", ObjectCodeAction::Freeze);
+    assert_success!(status);
+}
+
 #[test]
 fn freeze_code_object_fail_when_package_registry_does_not_exist() {
     let mut context = TestContext::new(None, None);

aptos-move/framework/aptos-framework/doc/code.md

@@ -94,7 +94,7 @@ The package registry at the given address.
 Metadata for a package. All byte blobs are represented as base64-of-gzipped-bytes
 
 
-<pre><code><b>struct</b> <a href="code.md#0x1_code_PackageMetadata">PackageMetadata</a> <b>has</b> drop, store
+<pre><code><b>struct</b> <a href="code.md#0x1_code_PackageMetadata">PackageMetadata</a> <b>has</b> <b>copy</b>, drop, store
 </code></pre>
 
 
@@ -200,7 +200,7 @@ A dependency to a package published at address
 Metadata about a module in a package.
 
 
-<pre><code><b>struct</b> <a href="code.md#0x1_code_ModuleMetadata">ModuleMetadata</a> <b>has</b> drop, store
+<pre><code><b>struct</b> <a href="code.md#0x1_code_ModuleMetadata">ModuleMetadata</a> <b>has</b> <b>copy</b>, drop, store
 </code></pre>
 
 
@@ -687,10 +687,18 @@ package.
     );
 
     <b>let</b> registry = <b>borrow_global_mut</b>&lt;<a href="code.md#0x1_code_PackageRegistry">PackageRegistry</a>&gt;(code_object_addr);
-    <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector_for_each_mut">vector::for_each_mut</a>&lt;<a href="code.md#0x1_code_PackageMetadata">PackageMetadata</a>&gt;(&<b>mut</b> registry.packages, |pack| {
+    <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector_for_each_mut">vector::for_each_mut</a>(&<b>mut</b> registry.packages, |pack| {
         <b>let</b> package: &<b>mut</b> <a href="code.md#0x1_code_PackageMetadata">PackageMetadata</a> = pack;
         package.upgrade_policy = <a href="code.md#0x1_code_upgrade_policy_immutable">upgrade_policy_immutable</a>();
     });
+
+    // We unfortunately have <b>to</b> make a <b>copy</b> of each package <b>to</b> avoid borrow checker issues <b>as</b> check_dependencies
+    // needs <b>to</b> borrow <a href="code.md#0x1_code_PackageRegistry">PackageRegistry</a> from the dependency packages.
+    // This would increase the amount of gas used, but this is a rare operation and it's rare <b>to</b> have many packages
+    // in a single <a href="code.md#0x1_code">code</a> <a href="object.md#0x1_object">object</a>.
+    <a href="../../aptos-stdlib/../move-stdlib/doc/vector.md#0x1_vector_for_each">vector::for_each</a>(registry.packages, |pack| {
+        <a href="code.md#0x1_code_check_dependencies">check_dependencies</a>(code_object_addr, &pack);
+    });
 }
 </code></pre>
 

aptos-move/framework/aptos-framework/sources/code.move

@@ -24,7 +24,7 @@ module aptos_framework::code {
     }
 
     /// Metadata for a package. All byte blobs are represented as base64-of-gzipped-bytes
-    struct PackageMetadata has store, drop {
+    struct PackageMetadata has copy, drop, store {
         /// Name of this package.
         name: String,
         /// The upgrade policy of this package.
@@ -52,7 +52,7 @@ module aptos_framework::code {
     }
 
     /// Metadata about a module in a package.
-    struct ModuleMetadata has store, drop {
+    struct ModuleMetadata has copy, drop, store {
         /// Name of the module.
         name: String,
         /// Source text, gzipped String. Empty if not provided.
@@ -214,10 +214,18 @@ module aptos_framework::code {
         );
 
         let registry = borrow_global_mut<PackageRegistry>(code_object_addr);
-        vector::for_each_mut<PackageMetadata>(&mut registry.packages, |pack| {
+        vector::for_each_mut(&mut registry.packages, |pack| {
             let package: &mut PackageMetadata = pack;
             package.upgrade_policy = upgrade_policy_immutable();
         });
+
+        // We unfortunately have to make a copy of each package to avoid borrow checker issues as check_dependencies
+        // needs to borrow PackageRegistry from the dependency packages.
+        // This would increase the amount of gas used, but this is a rare operation and it's rare to have many packages
+        // in a single code object.
+        vector::for_each(registry.packages, |pack| {
+            check_dependencies(code_object_addr, &pack);
+        });
     }
 
     /// Same as `publish_package` but as an entry function which can be called as a transaction. Because