fix metadata (#13236)

aptos-labs · May 9, 2024 · c90dafd · c90dafd
1 parent abe62d1
commit c90dafd
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 59 deletions.
diff --git a/aptos-move/e2e-move-tests/src/tests/metadata.rs b/aptos-move/e2e-move-tests/src/tests/metadata.rs
@@ -18,8 +18,10 @@ use aptos_types::{
 };
 use move_binary_format::CompiledModule;
 use move_core_types::{
-    account_address::AccountAddress, language_storage::CORE_CODE_ADDRESS, metadata::Metadata,
-    vm_status::StatusCode,
+    account_address::AccountAddress,
+    language_storage::CORE_CODE_ADDRESS,
+    metadata::Metadata,
+    vm_status::{StatusCode, StatusCode::CONSTRAINT_NOT_SATISFIED},
 };
 use move_model::metadata::{CompilationMetadata, CompilerVersion, COMPILATION_METADATA_KEY};
 use std::collections::BTreeMap;
@@ -275,13 +277,13 @@ fn test_compilation_metadata() {
         test_compilation_metadata_internal(true, true, enable_check),
         StatusCode::UNSTABLE_BYTECODE_REJECTED
     );
-    // publish compiler v2 code to test
+    // publish compiler v1 code to mainnet
     assert_success!(test_compilation_metadata_internal(
         true,
         false,
         enable_check
     ));
-    // publish compiler v1 code to mainnet
+    // publish compiler v2 code to test
     assert_success!(test_compilation_metadata_internal(
         false,
         true,
@@ -296,20 +298,23 @@ fn test_compilation_metadata() {
 
     enable_check = false;
     // publish compiler v2 code to mainnet
-    // success because the feature flag is disabled
-    assert_success!(test_compilation_metadata_internal(true, true, enable_check));
-    // publish compiler v2 code to test
-    assert_success!(test_compilation_metadata_internal(
-        true,
-        false,
-        enable_check
-    ));
+    // failed because the metadata cannot be recognized
+    assert_vm_status!(
+        test_compilation_metadata_internal(true, true, enable_check),
+        CONSTRAINT_NOT_SATISFIED
+    );
     // publish compiler v1 code to mainnet
     assert_success!(test_compilation_metadata_internal(
-        false,
         true,
+        false,
         enable_check
     ));
+    // publish compiler v2 code to test
+    // failed because the metadata cannot be recognized
+    assert_vm_status!(
+        test_compilation_metadata_internal(false, true, enable_check),
+        CONSTRAINT_NOT_SATISFIED
+    );
     // publish compiler v1 code to test
     assert_success!(test_compilation_metadata_internal(
         false,

diff --git a/aptos-move/framework/aptos-framework/doc/primary_fungible_store.md b/aptos-move/framework/aptos-framework/doc/primary_fungible_store.md
@@ -486,49 +486,7 @@ Transfer <code>amount</code> of fungible asset from sender's primary store to re
     // Check <b>if</b> the sender store <a href="object.md#0x1_object">object</a> <b>has</b> been burnt or not. If so, unburn it first.
     <a href="primary_fungible_store.md#0x1_primary_fungible_store_may_be_unburn">may_be_unburn</a>(sender, sender_store);
     <b>let</b> recipient_store = <a href="primary_fungible_store.md#0x1_primary_fungible_store_ensure_primary_store_exists">ensure_primary_store_exists</a>(recipient, metadata);
-    <a href="dispatchable_fungible_asset.md#0x1_dispatchable_fungible_asset_transfer">dispatchable_fungible_asset::transfer</a>(sender, sender_store, recipient_store, amount);
-}
-</code></pre>
-
-
-
-</details>
-
-<a id="0x1_primary_fungible_store_transfer_assert_minimum_deposit"></a>
-
-## Function `transfer_assert_minimum_deposit`
-
-Transfer <code>amount</code> of fungible asset from sender's primary store to receiver's primary store.
-Use the minimum deposit assertion api to make sure receipient will receive a minimum amount of fund.
-
-
-<pre><code><b>public</b> entry <b>fun</b> <a href="primary_fungible_store.md#0x1_primary_fungible_store_transfer_assert_minimum_deposit">transfer_assert_minimum_deposit</a>&lt;T: key&gt;(sender: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>, metadata: <a href="object.md#0x1_object_Object">object::Object</a>&lt;T&gt;, recipient: <b>address</b>, amount: u64, expected: u64)
-</code></pre>
-
-
-
-<details>
-<summary>Implementation</summary>
-
-
-<pre><code><b>public</b> entry <b>fun</b> <a href="primary_fungible_store.md#0x1_primary_fungible_store_transfer_assert_minimum_deposit">transfer_assert_minimum_deposit</a>&lt;T: key&gt;(
-    sender: &<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer">signer</a>,
-    metadata: Object&lt;T&gt;,
-    recipient: <b>address</b>,
-    amount: u64,
-    expected: u64,
-) <b>acquires</b> <a href="primary_fungible_store.md#0x1_primary_fungible_store_DeriveRefPod">DeriveRefPod</a> {
-    <b>let</b> sender_store = <a href="primary_fungible_store.md#0x1_primary_fungible_store_ensure_primary_store_exists">ensure_primary_store_exists</a>(<a href="../../aptos-stdlib/../move-stdlib/doc/signer.md#0x1_signer_address_of">signer::address_of</a>(sender), metadata);
-    // Check <b>if</b> the sender store <a href="object.md#0x1_object">object</a> <b>has</b> been burnt or not. If so, unburn it first.
-    <a href="primary_fungible_store.md#0x1_primary_fungible_store_may_be_unburn">may_be_unburn</a>(sender, sender_store);
-    <b>let</b> recipient_store = <a href="primary_fungible_store.md#0x1_primary_fungible_store_ensure_primary_store_exists">ensure_primary_store_exists</a>(recipient, metadata);
-    <a href="dispatchable_fungible_asset.md#0x1_dispatchable_fungible_asset_transfer_assert_minimum_deposit">dispatchable_fungible_asset::transfer_assert_minimum_deposit</a>(
-        sender,
-        sender_store,
-        recipient_store,
-        amount,
-        expected
-    );
+    <a href="fungible_asset.md#0x1_fungible_asset_transfer">fungible_asset::transfer</a>(sender, sender_store, recipient_store, amount);
 }
 </code></pre>
 

diff --git a/aptos-move/framework/src/module_metadata.rs b/aptos-move/framework/src/module_metadata.rs
@@ -224,7 +224,10 @@ pub fn get_vm_metadata_v0(
 }
 
 /// Check if the metadata has unknown key/data types
-pub fn check_metadata_format(module: &CompiledModule) -> Result<(), MalformedError> {
+pub fn check_metadata_format(
+    module: &CompiledModule,
+    features: &Features,
+) -> Result<(), MalformedError> {
     let mut exist = false;
     let mut compilation_key_exist = false;
     for data in module.metadata.iter() {
@@ -241,7 +244,9 @@ pub fn check_metadata_format(module: &CompiledModule) -> Result<(), MalformedErr
                 bcs::from_bytes::<RuntimeModuleMetadataV1>(&data.value)
                     .map_err(|e| MalformedError::DeserializedError(data.key.clone(), e))?;
             }
-        } else if data.key == *COMPILATION_METADATA_KEY {
+        } else if features.is_enabled(FeatureFlag::REJECT_UNSTABLE_BYTECODE)
+            && data.key == *COMPILATION_METADATA_KEY
+        {
             if compilation_key_exist {
                 return Err(MalformedError::DuplicateKey);
             }
@@ -450,7 +455,7 @@ pub fn verify_module_metadata(
     }
 
     if features.are_resource_groups_enabled() {
-        check_metadata_format(module)?;
+        check_metadata_format(module, features)?;
     }
     let metadata = if let Some(metadata) = get_metadata_from_compiled_module(module) {
         metadata