unit test and mark dirty

aptos-labs · Oct 16, 2024 · c5f381e · c5f381e
1 parent 4f1bc30
commit c5f381e
Show file tree

Hide file tree

Showing 5 changed files with 130 additions and 0 deletions.
diff --git a/aptos-move/aptos-release-builder/src/components/feature_flags.rs b/aptos-move/aptos-release-builder/src/components/feature_flags.rs
@@ -131,6 +131,7 @@ pub enum FeatureFlag {
     FederatedKeyless,
     TransactionSimulationEnhancement,
     CollectionOwner,
+    NativeMemoryOperations,
 }
 
 fn generate_features_blob(writer: &CodeWriter, data: &[u64]) {
@@ -347,6 +348,7 @@ impl From<FeatureFlag> for AptosFeatureFlag {
                 AptosFeatureFlag::TRANSACTION_SIMULATION_ENHANCEMENT
             },
             FeatureFlag::CollectionOwner => AptosFeatureFlag::COLLECTION_OWNER,
+            FeatureFlag::NativeMemoryOperations => AptosFeatureFlag::NATIVE_MEMORY_OPERATIONS,
         }
     }
 }
@@ -490,6 +492,7 @@ impl From<AptosFeatureFlag> for FeatureFlag {
                 FeatureFlag::TransactionSimulationEnhancement
             },
             AptosFeatureFlag::COLLECTION_OWNER => FeatureFlag::CollectionOwner,
+            AptosFeatureFlag::NATIVE_MEMORY_OPERATIONS => FeatureFlag::NativeMemoryOperations,
         }
     }
 }

diff --git a/aptos-move/framework/move-stdlib/src/natives/mem.rs b/aptos-move/framework/move-stdlib/src/natives/mem.rs
@@ -22,6 +22,16 @@ use move_vm_types::{
 use smallvec::{smallvec, SmallVec};
 use std::collections::VecDeque;
 
+/// The feature is not enabled.
+/// (0xD is unavailable)
+pub const EFEATURE_NOT_ENABLED: u64 = 0x0D_0001;
+
+pub fn get_feature_not_available_error() -> SafeNativeError {
+    SafeNativeError::Abort {
+        abort_code: EFEATURE_NOT_ENABLED,
+    }
+}
+
 /***************************************************************************************************
  * native fun native_swap
  *
@@ -33,6 +43,13 @@ fn native_swap(
     _ty_args: Vec<Type>,
     mut args: VecDeque<Value>,
 ) -> SafeNativeResult<SmallVec<[Value; 1]>> {
+    if !context
+        .get_feature_flags()
+        .is_native_memory_operations_enabled()
+    {
+        return Err(get_feature_not_available_error());
+    }
+
     debug_assert!(args.len() == 2);
 
     if args.len() != 2 {

diff --git a/third_party/move/move-vm/types/src/values/value_tests.rs b/third_party/move/move-vm/types/src/values/value_tests.rs
@@ -3,6 +3,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use crate::{loaded_data::runtime_types::TypeBuilder, values::*, views::*};
+use claims::{assert_err, assert_ok};
 use move_binary_format::errors::*;
 use move_core_types::{account_address::AccountAddress, u256::U256};
 
@@ -229,6 +230,105 @@ fn test_vm_value_vector_u64_casting() {
     );
 }
 
+#[test]
+fn test_mem_swap() -> PartialVMResult<()> {
+    let mut locals = Locals::new(20);
+    // IndexedRef(Locals)
+    locals.store_loc(0, Value::u64(0), false)?;
+    locals.store_loc(1, Value::u64(1), false)?;
+    locals.store_loc(2, Value::address(AccountAddress::ZERO), false)?;
+    locals.store_loc(3, Value::address(AccountAddress::ONE), false)?;
+
+    // ContainerRef
+
+    // - Specialized
+    locals.store_loc(4, Value::vector_u64(vec![1, 2]), false)?;
+    locals.store_loc(5, Value::vector_u64(vec![3, 4, 5]), false)?;
+    locals.store_loc(6, Value::vector_address(vec![AccountAddress::ZERO]), false)?;
+    locals.store_loc(7, Value::vector_address(vec![AccountAddress::ONE]), false)?;
+
+    // - Generic
+    // -- Container of container
+    locals.store_loc(8, Value::struct_(Struct::pack(vec![Value::u16(4)])), false)?;
+    locals.store_loc(9, Value::struct_(Struct::pack(vec![Value::u16(5)])), false)?;
+    locals.store_loc(10, Value::signer(AccountAddress::ZERO), false)?;
+    locals.store_loc(11, Value::signer(AccountAddress::ONE), false)?;
+
+    // -- Container of vector
+    locals.store_loc(
+        12,
+        Value::vector_for_testing_only(vec![Value::u64(1u64), Value::u64(2u64)]),
+        false,
+    )?;
+    locals.store_loc(
+        13,
+        Value::vector_for_testing_only(vec![Value::u64(3u64), Value::u64(4u64)]),
+        false,
+    )?;
+    locals.store_loc(
+        14,
+        Value::vector_for_testing_only(vec![Value::signer(AccountAddress::ZERO)]),
+        false,
+    )?;
+    locals.store_loc(
+        15,
+        Value::vector_for_testing_only(vec![Value::signer(AccountAddress::ONE)]),
+        false,
+    )?;
+
+    let mut locals2 = Locals::new(2);
+    locals2.store_loc(0, Value::u64(0), false)?;
+
+    let get_local =
+        |ls: &Locals, idx: usize| ls.borrow_loc(idx).unwrap().value_as::<Reference>().unwrap();
+
+    for i in (0..16).step_by(2) {
+        assert_ok!(get_local(&locals, i).swap_values(get_local(&locals, i + 1)));
+    }
+
+    assert_ok!(get_local(&locals, 0).swap_values(get_local(&locals2, 0)));
+
+    for i in (0..16).step_by(2) {
+        for j in ((i + 2)..16).step_by(2) {
+            let result = get_local(&locals, i).swap_values(get_local(&locals, j));
+
+            // These would all fail in `call_native` typing checks.
+            // But here some do pass:
+            if j < 4  // locals are not checked between each other
+               || (8 <= i && j < 12) // ContainerRef of containers is not checked between each other
+               || (12 <= i && j < 16)
+            // ContainerRef of vector is not checked between each other
+            //    || i >= 8 // containers are also interchangeable
+            {
+                assert_ok!(result, "{} and {}", i, j);
+            } else {
+                assert_err!(result, "{} and {}", i, j);
+            }
+        }
+    }
+
+    // assert_err!(get_local(&locals, 0).swap_values(get_local(&locals, 4)));
+    // assert_err!(get_local(&locals, 0).swap_values(get_local(&locals, 6)));
+    // assert_err!(get_local(&locals, 0).swap_values(get_local(&locals, 8)));
+    // assert_err!(get_local(&locals, 0).swap_values(get_local(&locals, 10)));
+    // assert_err!(get_local(&locals, 0).swap_values(get_local(&locals, 12)));
+    // assert_err!(get_local(&locals, 2).swap_values(get_local(&locals, 4)));
+    // assert_err!(get_local(&locals, 2).swap_values(get_local(&locals, 4)));
+    // assert_err!(get_local(&locals, 2).swap_values(get_local(&locals, 4)));
+    // assert_err!(get_local(&locals, 2).swap_values(get_local(&locals, 4)));
+
+    // assert_ok!(get_local(&locals, 0).swap_values(get_local(&locals, 1)));
+    // assert_ok!(get_local(&locals, 2).swap_values(get_local(&locals, 3)));
+    // assert_ok!(get_local(&locals, 4).swap_values(get_local(&locals, 5)));
+    // assert_ok!(get_local(&locals, 6).swap_values(get_local(&locals, 7)));
+    // assert_ok!(get_local(&locals, 8).swap_values(get_local(&locals, 9)));
+    // assert_ok!(get_local(&locals, 10).swap_values(get_local(&locals, 11)));
+    // assert_ok!(get_local(&locals, 12).swap_values(get_local(&locals, 13)));
+    // assert_ok!(get_local(&locals, 14).swap_values(get_local(&locals, 15)));
+
+    Ok(())
+}
+
 #[cfg(test)]
 mod native_values {
     use super::*;

diff --git a/third_party/move/move-vm/types/src/values/values_impl.rs b/third_party/move/move-vm/types/src/values/values_impl.rs
@@ -1099,6 +1099,9 @@ impl IndexedRef {
             },
         }
 
+        self.container_ref.mark_dirty();
+        other.container_ref.mark_dirty();
+
         Ok(())
     }
 }

diff --git a/types/src/on_chain_config/aptos_features.rs b/types/src/on_chain_config/aptos_features.rs
@@ -95,6 +95,8 @@ pub enum FeatureFlag {
     FEDERATED_KEYLESS = 77,
     TRANSACTION_SIMULATION_ENHANCEMENT = 78,
     COLLECTION_OWNER = 79,
+    /// covers mem::swap and vector::move_range
+    NATIVE_MEMORY_OPERATIONS = 80,
 }
 
 impl FeatureFlag {
@@ -172,6 +174,7 @@ impl FeatureFlag {
             FeatureFlag::ENABLE_RESOURCE_ACCESS_CONTROL,
             FeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT,
             FeatureFlag::TRANSACTION_SIMULATION_ENHANCEMENT,
+            FeatureFlag::NATIVE_MEMORY_OPERATIONS,
         ]
     }
 }
@@ -317,6 +320,10 @@ impl Features {
         self.is_enabled(FeatureFlag::TRANSACTION_SIMULATION_ENHANCEMENT)
     }
 
+    pub fn is_native_memory_operations_enabled(&self) -> bool {
+        self.is_enabled(FeatureFlag::NATIVE_MEMORY_OPERATIONS)
+    }
+
     pub fn get_max_identifier_size(&self) -> u64 {
         if self.is_enabled(FeatureFlag::LIMIT_MAX_IDENTIFIER_LENGTH) {
             IDENTIFIER_SIZE_MAX