diff --git a/Cargo.lock b/Cargo.lock index 3712ab7eec74b..4bf18fe479e52 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2888,6 +2888,7 @@ version = "0.1.1" dependencies = [ "aptos-gas-schedule", "aptos-native-interface", + "aptos-types", "dir-diff", "file_diff", "move-cli", diff --git a/aptos-move/aptos-gas-schedule/src/gas_schedule/move_stdlib.rs b/aptos-move/aptos-gas-schedule/src/gas_schedule/move_stdlib.rs index 846a593afa46c..ec6455e2ed17a 100644 --- a/aptos-move/aptos-gas-schedule/src/gas_schedule/move_stdlib.rs +++ b/aptos-move/aptos-gas-schedule/src/gas_schedule/move_stdlib.rs @@ -7,7 +7,9 @@ use crate::{ gas_feature_versions::{RELEASE_V1_18, RELEASE_V1_24}, gas_schedule::NativeGasParameters, }; -use aptos_gas_algebra::{InternalGas, InternalGasPerAbstractValueUnit, InternalGasPerByte}; +use aptos_gas_algebra::{ + InternalGas, InternalGasPerAbstractValueUnit, InternalGasPerArg, InternalGasPerByte, +}; crate::gas_schedule::macros::define_gas_parameters!( MoveStdlibGasParameters, @@ -42,5 +44,8 @@ crate::gas_schedule::macros::define_gas_parameters!( [cmp_compare_base: InternalGas, { RELEASE_V1_24.. => "cmp.compare.base" }, 367], [cmp_compare_per_abs_val_unit: InternalGasPerAbstractValueUnit, { RELEASE_V1_24.. => "cmp.compare.per_abs_val_unit"}, 14], + + [vector_move_range_base: InternalGas, { RELEASE_V1_24.. => "vector.move_range.base" }, 4000], + [vector_move_range_per_index_moved: InternalGasPerArg, { RELEASE_V1_24.. => "vector.move_range.per_index_moved" }, 10], ] ); diff --git a/aptos-move/aptos-gas-schedule/src/ver.rs b/aptos-move/aptos-gas-schedule/src/ver.rs index cb8fc432fe52f..9e40e1d833880 100644 --- a/aptos-move/aptos-gas-schedule/src/ver.rs +++ b/aptos-move/aptos-gas-schedule/src/ver.rs @@ -69,7 +69,7 @@ /// global operations. /// - V1 /// - TBA -pub const LATEST_GAS_FEATURE_VERSION: u64 = gas_feature_versions::RELEASE_V1_23; +pub const LATEST_GAS_FEATURE_VERSION: u64 = gas_feature_versions::RELEASE_V1_24; pub mod gas_feature_versions { pub const RELEASE_V1_8: u64 = 11; diff --git a/aptos-move/aptos-release-builder/src/components/feature_flags.rs b/aptos-move/aptos-release-builder/src/components/feature_flags.rs index 4cd1b2a9db29d..cf161ac8c8947 100644 --- 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, EnableLoaderV2, } @@ -348,6 +349,7 @@ impl From for AptosFeatureFlag { AptosFeatureFlag::TRANSACTION_SIMULATION_ENHANCEMENT }, FeatureFlag::CollectionOwner => AptosFeatureFlag::COLLECTION_OWNER, + FeatureFlag::NativeMemoryOperations => AptosFeatureFlag::NATIVE_MEMORY_OPERATIONS, FeatureFlag::EnableLoaderV2 => AptosFeatureFlag::ENABLE_LOADER_V2, } } @@ -492,6 +494,7 @@ impl From for FeatureFlag { FeatureFlag::TransactionSimulationEnhancement }, AptosFeatureFlag::COLLECTION_OWNER => FeatureFlag::CollectionOwner, + AptosFeatureFlag::NATIVE_MEMORY_OPERATIONS => FeatureFlag::NativeMemoryOperations, AptosFeatureFlag::ENABLE_LOADER_V2 => FeatureFlag::EnableLoaderV2, } } diff --git a/aptos-move/aptos-vm-environment/src/natives.rs b/aptos-move/aptos-vm-environment/src/natives.rs index 4e11d2f21e9cb..d138ca40c2d2d 100644 --- a/aptos-move/aptos-vm-environment/src/natives.rs +++ b/aptos-move/aptos-vm-environment/src/natives.rs @@ -4,16 +4,35 @@ use aptos_native_interface::SafeNativeBuilder; use move_core_types::language_storage::CORE_CODE_ADDRESS; use move_vm_runtime::native_functions::NativeFunctionTable; +use std::collections::HashSet; /// Builds and returns all Aptos native functions. pub fn aptos_natives_with_builder( builder: &mut SafeNativeBuilder, inject_create_signer_for_gov_sim: bool, ) -> NativeFunctionTable { + let vector_bytecode_instruction_methods = HashSet::from([ + "empty", + "length", + "borrow", + "borrow_mut", + "push_back", + "pop_back", + "destroy_empty", + "swap", + ]); + #[allow(unreachable_code)] aptos_move_stdlib::natives::all_natives(CORE_CODE_ADDRESS, builder) .into_iter() - .filter(|(_, name, _, _)| name.as_str() != "vector") + .filter(|(_, name, func_name, _)| + if name.as_str() == "vector" && vector_bytecode_instruction_methods.contains(func_name.as_str()) { + println!("ERROR: Tried to register as native a vector bytecode_instruction method {}, skipping.", func_name.as_str()); + false + } else { + true + } + ) .chain(aptos_framework::natives::all_natives( CORE_CODE_ADDRESS, builder, diff --git a/aptos-move/framework/move-stdlib/Cargo.toml b/aptos-move/framework/move-stdlib/Cargo.toml index e02f9290530f3..3fb0a928cd8d9 100644 --- a/aptos-move/framework/move-stdlib/Cargo.toml +++ b/aptos-move/framework/move-stdlib/Cargo.toml @@ -13,6 +13,7 @@ publish = false [dependencies] aptos-gas-schedule = { workspace = true } aptos-native-interface = { workspace = true } +aptos-types = { workspace = true } move-core-types = { path = "../../../third_party/move/move-core/types" } move-vm-runtime = { path = "../../../third_party/move/move-vm/runtime" } move-vm-types = { path = "../../../third_party/move/move-vm/types" } diff --git a/aptos-move/framework/move-stdlib/doc/vector.md b/aptos-move/framework/move-stdlib/doc/vector.md index d5e6a7bfa2ecd..6507ce116f6ec 100644 --- a/aptos-move/framework/move-stdlib/doc/vector.md +++ b/aptos-move/framework/move-stdlib/doc/vector.md @@ -24,6 +24,7 @@ the return on investment didn't seem worth it for these simple functions. - [Function `pop_back`](#0x1_vector_pop_back) - [Function `destroy_empty`](#0x1_vector_destroy_empty) - [Function `swap`](#0x1_vector_swap) +- [Function `move_range`](#0x1_vector_move_range) - [Function `singleton`](#0x1_vector_singleton) - [Function `reverse`](#0x1_vector_reverse) - [Function `reverse_slice`](#0x1_vector_reverse_slice) @@ -340,6 +341,35 @@ Aborts if i or j is out of bounds. + + + + +## Function `move_range` + +Moves range of elements [removal_position, removal_position + length) from vector from, +to vector to, inserting them starting at the insert_position. +In the from vector, elements after the selected range are moved left to fill the hole +(i.e. range is removed, while the order of the rest of the elements is kept) +In the to vector, elements after the insert_position are moved to the right to make space for new elements +(i.e. range is inserted, while the order of the rest of the elements is kept). +Move prevents from having two mutable references to the same value, so from and to vectors are always distinct. + + +
public(friend) fun move_range<T>(from: &mut vector<T>, removal_position: u64, length: u64, to: &mut vector<T>, insert_position: u64)
+
+ + + +
+Implementation + + +
native public(friend) fun move_range<T>(from: &mut vector<T>, removal_position: u64, length: u64, to: &mut vector<T>, insert_position: u64);
+
+ + +
diff --git a/aptos-move/framework/move-stdlib/sources/vector.move b/aptos-move/framework/move-stdlib/sources/vector.move index 1f7754a2a4cb8..f4a443d3fffe9 100644 --- a/aptos-move/framework/move-stdlib/sources/vector.move +++ b/aptos-move/framework/move-stdlib/sources/vector.move @@ -61,6 +61,20 @@ module std::vector { /// Aborts if `i` or `j` is out of bounds. native public fun swap(self: &mut vector, i: u64, j: u64); + // TODO - functions here are `public(friend)` here for one release, + // and to be changed to `public` one release later. + #[test_only] + friend std::vector_tests; + + /// Moves range of elements `[removal_position, removal_position + length)` from vector `from`, + /// to vector `to`, inserting them starting at the `insert_position`. + /// In the `from` vector, elements after the selected range are moved left to fill the hole + /// (i.e. range is removed, while the order of the rest of the elements is kept) + /// In the `to` vector, elements after the `insert_position` are moved to the right to make space for new elements + /// (i.e. range is inserted, while the order of the rest of the elements is kept). + /// Move prevents from having two mutable references to the same value, so `from` and `to` vectors are always distinct. + native public(friend) fun move_range(from: &mut vector, removal_position: u64, length: u64, to: &mut vector, insert_position: u64); + /// Return an vector of size one containing element `e`. public fun singleton(e: Element): vector { let v = empty(); diff --git a/aptos-move/framework/move-stdlib/src/natives/mod.rs b/aptos-move/framework/move-stdlib/src/natives/mod.rs index daf6d0cce0484..38e0f78119405 100644 --- a/aptos-move/framework/move-stdlib/src/natives/mod.rs +++ b/aptos-move/framework/move-stdlib/src/natives/mod.rs @@ -12,6 +12,7 @@ pub mod signer; pub mod string; #[cfg(feature = "testing")] pub mod unit_test; +pub mod vector; use aptos_native_interface::SafeNativeBuilder; use move_core_types::account_address::AccountAddress; @@ -37,6 +38,7 @@ pub fn all_natives( add_natives!("hash", hash::make_all(builder)); add_natives!("signer", signer::make_all(builder)); add_natives!("string", string::make_all(builder)); + add_natives!("vector", vector::make_all(builder)); #[cfg(feature = "testing")] { add_natives!("unit_test", unit_test::make_all(builder)); diff --git a/aptos-move/framework/move-stdlib/src/natives/vector.rs b/aptos-move/framework/move-stdlib/src/natives/vector.rs new file mode 100644 index 0000000000000..8719a2eb50174 --- /dev/null +++ b/aptos-move/framework/move-stdlib/src/natives/vector.rs @@ -0,0 +1,116 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +// Copyright (c) The Diem Core Contributors +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + +//! Implementation of native functions (non-bytecode instructions) for vector. + +use aptos_gas_schedule::gas_params::natives::move_stdlib::{ + VECTOR_MOVE_RANGE_BASE, VECTOR_MOVE_RANGE_PER_INDEX_MOVED, +}; +use aptos_native_interface::{ + safely_pop_arg, RawSafeNative, SafeNativeBuilder, SafeNativeContext, SafeNativeError, + SafeNativeResult, +}; +use aptos_types::error; +use move_core_types::gas_algebra::NumArgs; +use move_vm_runtime::native_functions::NativeFunction; +use move_vm_types::{ + loaded_data::runtime_types::Type, + values::{Value, VectorRef}, +}; +use smallvec::{smallvec, SmallVec}; +use std::collections::VecDeque; + +/// Given input positions/lengths are outside of vector boundaries. +pub const EINDEX_OUT_OF_BOUNDS: u64 = 1; + +/// The feature is not enabled. +pub const EFEATURE_NOT_ENABLED: u64 = 2; + +/*************************************************************************************************** + * native fun move_range(from: &mut vector, removal_position: u64, length: u64, to: &mut vector, insert_position: u64) + * + * gas cost: VECTOR_MOVE_RANGE_BASE + VECTOR_MOVE_RANGE_PER_INDEX_MOVED * num_elements_to_move + * + **************************************************************************************************/ +fn native_move_range( + context: &mut SafeNativeContext, + ty_args: Vec, + mut args: VecDeque, +) -> SafeNativeResult> { + if !context + .get_feature_flags() + .is_native_memory_operations_enabled() + { + return Err(SafeNativeError::Abort { + abort_code: error::unavailable(EFEATURE_NOT_ENABLED), + }); + } + + context.charge(VECTOR_MOVE_RANGE_BASE)?; + + let map_err = |_| SafeNativeError::Abort { + abort_code: error::invalid_argument(EINDEX_OUT_OF_BOUNDS), + }; + let insert_position = usize::try_from(safely_pop_arg!(args, u64)).map_err(map_err)?; + let to = safely_pop_arg!(args, VectorRef); + let length = usize::try_from(safely_pop_arg!(args, u64)).map_err(map_err)?; + let removal_position = usize::try_from(safely_pop_arg!(args, u64)).map_err(map_err)?; + let from = safely_pop_arg!(args, VectorRef); + + // We need to charge before executing, so fetching and checking sizes here. + // We repeat fetching and checking of the sizes inside VectorRef::move_range call as well. + // Not sure if possible to combine (as we are never doing charging there). + let to_len = to.length_as_usize(&ty_args[0])?; + let from_len = from.length_as_usize(&ty_args[0])?; + + if removal_position + .checked_add(length) + .map_or(true, |end| end > from_len) + || insert_position > to_len + { + return Err(SafeNativeError::Abort { + abort_code: EINDEX_OUT_OF_BOUNDS, + }); + } + + // We are moving all elements in the range, all elements after range, and all elements after insertion point. + // We are counting "length" of moving block twice, as it both gets moved out and moved in. + // From calibration testing, this seems to be a reasonable approximation of the cost of the operation. + context.charge( + VECTOR_MOVE_RANGE_PER_INDEX_MOVED + * NumArgs::new( + (from_len - removal_position) + .checked_add(to_len - insert_position) + .and_then(|v| v.checked_add(length)) + .ok_or_else(|| SafeNativeError::Abort { + abort_code: EINDEX_OUT_OF_BOUNDS, + })? as u64, + ), + )?; + + VectorRef::move_range( + &from, + removal_position, + length, + &to, + insert_position, + &ty_args[0], + )?; + + Ok(smallvec![]) +} + +/*************************************************************************************************** + * module + **************************************************************************************************/ +pub fn make_all( + builder: &SafeNativeBuilder, +) -> impl Iterator + '_ { + let natives = [("move_range", native_move_range as RawSafeNative)]; + + builder.make_named_natives(natives) +} diff --git a/aptos-move/framework/move-stdlib/tests/vector_tests.move b/aptos-move/framework/move-stdlib/tests/vector_tests.move index b8c9e19a4dd84..ce558bc3e9cfc 100644 --- a/aptos-move/framework/move-stdlib/tests/vector_tests.move +++ b/aptos-move/framework/move-stdlib/tests/vector_tests.move @@ -954,4 +954,14 @@ module std::vector_tests { let v = vector[MoveOnly {}]; vector::destroy(v, |m| { let MoveOnly {} = m; }) } + + #[test] + fun test_move_range_ints() { + let v = vector[3, 4, 5, 6]; + let w = vector[1, 2]; + + V::move_range(&mut v, 1, 2, &mut w, 1); + assert!(&v == &vector[3, 6], 0); + assert!(&w == &vector[1, 4, 5, 2], 0); + } } 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 index 4022290b38c7d..6626d9bbc2cde 100644 --- a/third_party/move/move-vm/types/src/values/values_impl.rs +++ b/third_party/move/move-vm/types/src/values/values_impl.rs @@ -2295,7 +2295,7 @@ fn check_elem_layout(ty: &Type, v: &Container) -> PartialVMResult<()> { } impl VectorRef { - pub fn len(&self, type_param: &Type) -> PartialVMResult { + pub fn length_as_usize(&self, type_param: &Type) -> PartialVMResult { let c: &Container = self.0.container(); check_elem_layout(type_param, c)?; @@ -2311,7 +2311,11 @@ impl VectorRef { Container::Vec(r) => r.borrow().len(), Container::Locals(_) | Container::Struct(_) => unreachable!(), }; - Ok(Value::u64(len as u64)) + Ok(len) + } + + pub fn len(&self, type_param: &Type) -> PartialVMResult { + Ok(Value::u64(self.length_as_usize(type_param)? as u64)) } pub fn push_back(&self, e: Value, type_param: &Type) -> PartialVMResult<()> { @@ -2440,6 +2444,78 @@ impl VectorRef { self.0.mark_dirty(); Ok(()) } + + /// Moves range of elements `[removal_position, removal_position + length)` from vector `from`, + /// to vector `to`, inserting them starting at the `insert_position`. + /// In the `from` vector, elements after the selected range are moved left to fill the hole + /// (i.e. range is removed, while the order of the rest of the elements is kept) + /// In the `to` vector, elements after the `insert_position` are moved to the right to make space for new elements + /// (i.e. range is inserted, while the order of the rest of the elements is kept). + /// + /// Precondition for this function is that `from` and `to` vectors are required to be distinct + /// Move will guaranteee that invariant, because it prevents from having two mutable references to the same value. + pub fn move_range( + from_self: &Self, + removal_position: usize, + length: usize, + to_self: &Self, + insert_position: usize, + type_param: &Type, + ) -> PartialVMResult<()> { + let from_c = from_self.0.container(); + let to_c = to_self.0.container(); + + // potentially unnecessary as native call should've checked the types already + // (unlike other vector functions that are bytecodes) + // TODO: potentially unnecessary, can be removed - as these are only required for + // bytecode instructions, as types are checked when native functions are called. + check_elem_layout(type_param, from_c)?; + check_elem_layout(type_param, to_c)?; + + macro_rules! move_range { + ($from:expr, $to:expr) => {{ + let mut from_v = $from.borrow_mut(); + let mut to_v = $to.borrow_mut(); + + if removal_position.checked_add(length).map_or(true, |end| end > from_v.len()) + || insert_position > to_v.len() { + return Err(PartialVMError::new(StatusCode::VECTOR_OPERATION_ERROR) + .with_sub_status(INDEX_OUT_OF_BOUNDS)); + } + + // Short-circuit with faster implementation some of the common cases. + // This includes all non-direct calls to move-range (i.e. insert/remove/append/split_off inside vector). + if length == 1 { + to_v.insert(insert_position, from_v.remove(removal_position)); + } else if removal_position == 0 && length == from_v.len() && insert_position == to_v.len() { + to_v.append(&mut from_v); + } else if (removal_position + length == from_v.len() && insert_position == to_v.len()) { + to_v.append(&mut from_v.split_off(removal_position)); + } else { + to_v.splice(insert_position..insert_position, from_v.splice(removal_position..(removal_position + length), [])); + } + }}; + } + + match (from_c, to_c) { + (Container::VecU8(from_r), Container::VecU8(to_r)) => move_range!(from_r, to_r), + (Container::VecU16(from_r), Container::VecU16(to_r)) => move_range!(from_r, to_r), + (Container::VecU32(from_r), Container::VecU32(to_r)) => move_range!(from_r, to_r), + (Container::VecU64(from_r), Container::VecU64(to_r)) => move_range!(from_r, to_r), + (Container::VecU128(from_r), Container::VecU128(to_r)) => move_range!(from_r, to_r), + (Container::VecU256(from_r), Container::VecU256(to_r)) => move_range!(from_r, to_r), + (Container::VecBool(from_r), Container::VecBool(to_r)) => move_range!(from_r, to_r), + (Container::VecAddress(from_r), Container::VecAddress(to_r)) => { + move_range!(from_r, to_r) + }, + (Container::Vec(from_r), Container::Vec(to_r)) => move_range!(from_r, to_r), + (_, _) => unreachable!(), + } + + from_self.0.mark_dirty(); + to_self.0.mark_dirty(); + Ok(()) + } } impl Vector { diff --git a/types/src/on_chain_config/aptos_features.rs b/types/src/on_chain_config/aptos_features.rs index d1297014ad21f..7ee0d4d2dbbd5 100644 --- a/types/src/on_chain_config/aptos_features.rs +++ b/types/src/on_chain_config/aptos_features.rs @@ -96,6 +96,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, ENABLE_LOADER_V2 = 81, } @@ -174,6 +176,7 @@ impl FeatureFlag { FeatureFlag::ENABLE_RESOURCE_ACCESS_CONTROL, FeatureFlag::REJECT_UNSTABLE_BYTECODE_FOR_SCRIPT, FeatureFlag::TRANSACTION_SIMULATION_ENHANCEMENT, + FeatureFlag::NATIVE_MEMORY_OPERATIONS, // TODO(loader_v2): Enable V2 loader. // FeatureFlag::ENABLE_LOADER_V2, ] @@ -320,6 +323,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 is_loader_v2_enabled(&self) -> bool { self.is_enabled(FeatureFlag::ENABLE_LOADER_V2) }