diff --git a/aptos-move/aptos-vm-types/src/environment.rs b/aptos-move/aptos-vm-types/src/environment.rs index 1b97652f05d2d..618676f8d7034 100644 --- a/aptos-move/aptos-vm-types/src/environment.rs +++ b/aptos-move/aptos-vm-types/src/environment.rs @@ -17,28 +17,23 @@ use std::sync::Arc; // TODO(George): move configs here from types crate. pub fn aptos_prod_ty_builder( - features: &Features, gas_feature_version: u64, gas_params: &AptosGasParameters, ) -> TypeBuilder { - if features.is_limit_type_size_enabled() && gas_feature_version >= RELEASE_V1_15 { + if gas_feature_version >= RELEASE_V1_15 { let max_ty_size = gas_params.vm.txn.max_ty_size; let max_ty_depth = gas_params.vm.txn.max_ty_depth; TypeBuilder::with_limits(max_ty_size.into(), max_ty_depth.into()) } else { - aptos_default_ty_builder(features) + aptos_default_ty_builder() } } -pub fn aptos_default_ty_builder(features: &Features) -> TypeBuilder { - if features.is_limit_type_size_enabled() { - // Type builder to use when: - // 1. Type size gas parameters are not yet in gas schedule (before V14). - // 2. No gas parameters are found on-chain. - TypeBuilder::with_limits(128, 20) - } else { - TypeBuilder::Legacy - } +pub fn aptos_default_ty_builder() -> TypeBuilder { + // Type builder to use when: + // 1. Type size gas parameters are not yet in gas schedule (before 1.15). + // 2. No gas parameters are found on-chain. + TypeBuilder::with_limits(128, 20) } /// A runtime environment which can be used for VM initialization and more. @@ -68,7 +63,7 @@ impl Environment { } let timed_features = timed_features_builder.build(); - let ty_builder = aptos_default_ty_builder(&features); + let ty_builder = aptos_default_ty_builder(); Self::initialize(features, timed_features, chain_id, ty_builder) } @@ -80,7 +75,7 @@ impl Environment { .with_override_profile(TimedFeatureOverride::Testing) .build(); - let ty_builder = aptos_default_ty_builder(&features); + let ty_builder = aptos_default_ty_builder(); Arc::new(Self::initialize( features, timed_features, diff --git a/aptos-move/aptos-vm/src/move_vm_ext/vm.rs b/aptos-move/aptos-vm/src/move_vm_ext/vm.rs index 443829ee16ef2..a4489da1031ad 100644 --- a/aptos-move/aptos-vm/src/move_vm_ext/vm.rs +++ b/aptos-move/aptos-vm/src/move_vm_ext/vm.rs @@ -45,7 +45,7 @@ impl GenesisMoveVM { &features, &timed_features, pseudo_meter_vector_ty_to_ty_tag_construction, - aptos_default_ty_builder(&features), + aptos_default_ty_builder(), ); // All genesis sessions run with unmetered gas meter, and here we set @@ -113,8 +113,7 @@ impl MoveVmExt { // We should clean up the logic here once we get that refactored. let (native_gas_params, misc_gas_params, ty_builder) = match gas_params { Ok(gas_params) => { - let ty_builder = - aptos_prod_ty_builder(env.features(), gas_feature_version, gas_params); + let ty_builder = aptos_prod_ty_builder(gas_feature_version, gas_params); ( gas_params.natives.clone(), gas_params.vm.misc.clone(), @@ -122,7 +121,7 @@ impl MoveVmExt { ) }, Err(_) => { - let ty_builder = aptos_default_ty_builder(env.features()); + let ty_builder = aptos_default_ty_builder(); ( NativeGasParameters::zeros(), MiscGasParameters::zeros(), diff --git a/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs b/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs index 2e60d1cefb188..b4be0a60880de 100644 --- a/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs +++ b/aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs @@ -135,12 +135,7 @@ pub fn validate_combine_signer_and_txn_args( // Need to keep this here to ensure we return the historic correct error code for replay for ty in func.param_tys()[signer_param_cnt..].iter() { let subst_res = ty_builder.create_ty_with_subst(ty, func.ty_args()); - let ty = if ty_builder.is_legacy() { - subst_res.unwrap() - } else { - subst_res.map_err(|e| e.finish(Location::Undefined).into_vm_status())? - }; - + let ty = subst_res.map_err(|e| e.finish(Location::Undefined).into_vm_status())?; let valid = is_valid_txn_arg(session, &ty, allowed_structs); if !valid { return Err(VMStatus::error( @@ -235,12 +230,7 @@ pub(crate) fn construct_args( let ty_builder = session.get_ty_builder(); for (ty, arg) in types.iter().zip(args) { let subst_res = ty_builder.create_ty_with_subst(ty, ty_args); - let ty = if ty_builder.is_legacy() { - subst_res.unwrap() - } else { - subst_res.map_err(|e| e.finish(Location::Undefined).into_vm_status())? - }; - + let ty = subst_res.map_err(|e| e.finish(Location::Undefined).into_vm_status())?; let arg = construct_arg(session, &ty, allowed_structs, arg, &mut gas_meter, is_view)?; res_args.push(arg); } diff --git a/third_party/move/move-vm/integration-tests/src/tests/instantiation_tests.rs b/third_party/move/move-vm/integration-tests/src/tests/instantiation_tests.rs index 31f798569eb25..f67445dc76bcd 100644 --- a/third_party/move/move-vm/integration-tests/src/tests/instantiation_tests.rs +++ b/third_party/move/move-vm/integration-tests/src/tests/instantiation_tests.rs @@ -14,11 +14,7 @@ use move_core_types::{ language_storage::{StructTag, TypeTag}, vm_status::StatusCode, }; -use move_vm_runtime::{ - config::VMConfig, - module_traversal::{TraversalContext, TraversalStorage}, - move_vm::MoveVM, -}; +use move_vm_runtime::{config::VMConfig, move_vm::MoveVM}; use move_vm_test_utils::InMemoryStorage; use move_vm_types::gas::UnmeteredGasMeter; @@ -115,7 +111,6 @@ fn instantiation_err() { let mut session = vm.new_session(&storage); let mut mod_bytes = vec![]; cm.serialize(&mut mod_bytes).unwrap(); - let traversal_storage = TraversalStorage::new(); session .publish_module(mod_bytes, addr, &mut UnmeteredGasMeter) @@ -123,7 +118,6 @@ fn instantiation_err() { let mut ty_arg = TypeTag::U128; for _ in 0..4 { - // ty_arg = TypeTag::Vector(Box::new(ty_arg)); ty_arg = TypeTag::Struct(Box::new(StructTag { address: addr, module: Identifier::new("m").unwrap(), @@ -133,18 +127,12 @@ fn instantiation_err() { } let res = session.load_function(&cm.self_id(), ident_str!("f"), &[ty_arg]); - assert!(res.is_ok()); - let function = res.unwrap(); - - let err = session.execute_entry_function( - function, - Vec::>::new(), - &mut UnmeteredGasMeter, - &mut TraversalContext::new(&traversal_storage), + assert!( + res.is_err(), + "Instantiation must fail at load time when converting from type tag to type " ); - assert!(err.is_err(), "Instantiation must fail at runtime"); assert_eq!( - err.err().unwrap().major_status(), - StatusCode::VERIFICATION_ERROR + res.err().unwrap().major_status(), + StatusCode::TOO_MANY_TYPE_NODES ); } diff --git a/third_party/move/move-vm/runtime/src/config.rs b/third_party/move/move-vm/runtime/src/config.rs index 992f31390e15d..0e53320181669 100644 --- a/third_party/move/move-vm/runtime/src/config.rs +++ b/third_party/move/move-vm/runtime/src/config.rs @@ -41,7 +41,7 @@ impl Default for VMConfig { type_byte_cost: 0, pseudo_meter_vector_ty_to_ty_tag_construction: true, delayed_field_optimization_enabled: false, - ty_builder: TypeBuilder::Legacy, + ty_builder: TypeBuilder::with_limits(128, 20), disallow_dispatch_for_native: true, } } diff --git a/third_party/move/move-vm/runtime/src/interpreter.rs b/third_party/move/move-vm/runtime/src/interpreter.rs index df496c2850878..f5c5ed418c189 100644 --- a/third_party/move/move-vm/runtime/src/interpreter.rs +++ b/third_party/move/move-vm/runtime/src/interpreter.rs @@ -365,7 +365,7 @@ impl Interpreter { if !ty_args.is_empty() { let expected_ty = loader .ty_builder() - .create_ty_with_subst_with_legacy_check(expected_ty, &ty_args)?; + .create_ty_with_subst(expected_ty, &ty_args)?; ty.paranoid_check_eq(&expected_ty)?; } else { // Directly check against the expected type to save a clone here. @@ -399,11 +399,7 @@ impl Interpreter { function .local_tys() .iter() - .map(|ty| { - loader - .ty_builder() - .create_ty_with_subst_with_legacy_check(ty, &ty_args) - }) + .map(|ty| loader.ty_builder().create_ty_with_subst(ty, &ty_args)) .collect::>>()? } } else { @@ -488,8 +484,7 @@ impl Interpreter { let ty = self.operand_stack.pop_ty()?; let expected_ty = &function.param_tys()[expected_args - i - 1]; if !ty_args.is_empty() { - let expected_ty = - ty_builder.create_ty_with_subst_with_legacy_check(expected_ty, &ty_args)?; + let expected_ty = ty_builder.create_ty_with_subst(expected_ty, &ty_args)?; ty.paranoid_check_eq(&expected_ty)?; } else { ty.paranoid_check_eq(expected_ty)?; @@ -546,7 +541,7 @@ impl Interpreter { if self.paranoid_type_checks { for ty in function.return_tys() { - let ty = ty_builder.create_ty_with_subst_with_legacy_check(ty, &ty_args)?; + let ty = ty_builder.create_ty_with_subst(ty, &ty_args)?; self.operand_stack.push_ty(ty)?; } } diff --git a/third_party/move/move-vm/runtime/src/loader/mod.rs b/third_party/move/move-vm/runtime/src/loader/mod.rs index 18e5cbfa714be..2be6e1c54bb99 100644 --- a/third_party/move/move-vm/runtime/src/loader/mod.rs +++ b/third_party/move/move-vm/runtime/src/loader/mod.rs @@ -53,7 +53,7 @@ mod type_loader; pub use function::LoadedFunction; pub(crate) use function::{Function, FunctionHandle, FunctionInstantiation, Scope}; pub(crate) use modules::{Module, ModuleCache, ModuleStorage, ModuleStorageAdapter}; -use move_vm_types::loaded_data::runtime_types::{legacy_count_type_nodes, TypeBuilder}; +use move_vm_types::loaded_data::runtime_types::TypeBuilder; pub(crate) use script::{Script, ScriptCache}; use type_loader::intern_type; @@ -325,20 +325,6 @@ impl Loader { .map(|ty| self.load_type(ty, data_store, module_store)) .collect::>>()?; - #[allow(clippy::collapsible_if)] - if self.ty_builder().is_legacy() { - if ty_args.iter().map(legacy_count_type_nodes).sum::() - > TypeBuilder::LEGACY_MAX_TYPE_INSTANTIATION_NODES - { - return Err(PartialVMError::new(StatusCode::TOO_MANY_TYPE_NODES) - .with_message(format!( - "Too many type nodes when instantiating a type for script {}", - &main.name - )) - .finish(Location::Script)); - }; - } - Type::verify_ty_arg_abilities(main.ty_param_abilities(), &ty_args).map_err(|e| { e.with_message(format!( "Failed to verify type arguments for script {}", @@ -1218,22 +1204,9 @@ impl<'a> Resolver<'a> { let ty_builder = self.loader().ty_builder(); let mut instantiation = vec![]; for ty in &func_inst.instantiation { - let ty = ty_builder.create_ty_with_subst_with_legacy_check(ty, ty_args)?; + let ty = ty_builder.create_ty_with_subst(ty, ty_args)?; instantiation.push(ty); } - - if ty_builder.is_legacy() { - // Check if the function instantiation over all generics is larger - // than MAX_TYPE_INSTANTIATION_NODES. - let mut sum_nodes = 1u64; - for ty in ty_args.iter().chain(instantiation.iter()) { - sum_nodes = sum_nodes.saturating_add(legacy_count_type_nodes(ty)); - if sum_nodes > TypeBuilder::LEGACY_MAX_TYPE_INSTANTIATION_NODES { - return Err(PartialVMError::new(StatusCode::TOO_MANY_TYPE_NODES)); - } - } - } - Ok(instantiation) } @@ -1262,28 +1235,9 @@ impl<'a> Resolver<'a> { BinaryType::Script(_) => unreachable!("Scripts cannot have type instructions"), }; - let ty_builder = self.loader().ty_builder(); - if ty_builder.is_legacy() { - let mut sum_nodes = 1u64; - for ty in ty_args.iter().chain(struct_inst.instantiation.iter()) { - sum_nodes = sum_nodes.saturating_add(legacy_count_type_nodes(ty)); - if sum_nodes > TypeBuilder::LEGACY_MAX_TYPE_INSTANTIATION_NODES { - return Err( - PartialVMError::new(StatusCode::TOO_MANY_TYPE_NODES).with_message(format!( - "Number of type instantiation nodes exceeded the maximum of {}", - TypeBuilder::LEGACY_MAX_TYPE_INSTANTIATION_NODES - )), - ); - } - } - } - let struct_ty = &struct_inst.definition_struct_type; - ty_builder.create_struct_instantiation_ty_with_legacy_check( - struct_ty, - &struct_inst.instantiation, - ty_args, - ) + let ty_builder = self.loader().ty_builder(); + ty_builder.create_struct_instantiation_ty(struct_ty, &struct_inst.instantiation, ty_args) } pub(crate) fn get_field_ty(&self, idx: FieldHandleIndex) -> PartialVMResult<&Type> { @@ -1368,9 +1322,7 @@ impl<'a> Resolver<'a> { let ty = self.single_type_at(idx); if !ty_args.is_empty() { - self.loader() - .ty_builder() - .create_ty_with_subst_with_legacy_check(ty, ty_args) + self.loader().ty_builder().create_ty_with_subst(ty, ty_args) } else { Ok(ty.clone()) } @@ -1661,10 +1613,7 @@ impl Loader { let field_tys = struct_type .field_tys .iter() - .map(|ty| { - self.ty_builder() - .create_ty_with_subst_with_legacy_check(ty, ty_args) - }) + .map(|ty| self.ty_builder().create_ty_with_subst(ty, ty_args)) .collect::>>()?; let (mut field_layouts, field_has_identifier_mappings): (Vec, Vec) = field_tys @@ -1877,9 +1826,7 @@ impl Loader { .iter() .zip(&struct_type.field_tys) .map(|(n, ty)| { - let ty = self - .ty_builder() - .create_ty_with_subst_with_legacy_check(ty, ty_args)?; + let ty = self.ty_builder().create_ty_with_subst(ty, ty_args)?; let l = self.type_to_fully_annotated_layout_impl(&ty, module_store, count, depth)?; Ok(MoveFieldLayout::new(n.clone(), l)) diff --git a/third_party/move/move-vm/transactional-tests/tests/recursion/runtime_type_deeply_nested.exp b/third_party/move/move-vm/transactional-tests/tests/recursion/runtime_type_deeply_nested.exp index b37da7a441ce2..7f266bea7cd9a 100644 --- a/third_party/move/move-vm/transactional-tests/tests/recursion/runtime_type_deeply_nested.exp +++ b/third_party/move/move-vm/transactional-tests/tests/recursion/runtime_type_deeply_nested.exp @@ -2,18 +2,18 @@ processed 3 tasks task 1 'run'. lines 72-79: Error: Script execution failed with VMError: { - major_status: TOO_MANY_TYPE_NODES, + major_status: VM_MAX_TYPE_DEPTH_REACHED, sub_status: None, location: 0x42::M, indices: [], - offsets: [(FunctionDefinitionIndex(48), 0)], + offsets: [(FunctionDefinitionIndex(60), 0)], } task 2 'run'. lines 81-89: Error: Script execution failed with VMError: { - major_status: TOO_MANY_TYPE_NODES, + major_status: VM_MAX_TYPE_DEPTH_REACHED, sub_status: None, location: 0x42::M, indices: [], - offsets: [(FunctionDefinitionIndex(49), 0)], + offsets: [(FunctionDefinitionIndex(60), 0)], } diff --git a/third_party/move/move-vm/types/src/loaded_data/runtime_types.rs b/third_party/move/move-vm/types/src/loaded_data/runtime_types.rs index 918cbe782d2fb..0d03824fa07b4 100644 --- a/third_party/move/move-vm/types/src/loaded_data/runtime_types.rs +++ b/third_party/move/move-vm/types/src/loaded_data/runtime_types.rs @@ -692,38 +692,21 @@ impl fmt::Display for Type { /// Controls creation of runtime types, i.e., methods offered by this struct /// should be the only way to construct any type. #[derive(Clone, Serialize)] -pub enum TypeBuilder { - // Legacy configurations only limits type depth during type substitution. Will - // be removed in the nearest future when the new one stabilizes. The enum can - // be converted into the struct at that point. - Legacy, - WithLimits { - // Maximum number of nodes a fully-instantiated type has. - max_ty_size: u64, - // Maximum depth (in terms of number of nodes) a fully-instantiated type has. - max_ty_depth: u64, - }, +pub struct TypeBuilder { + // Maximum number of nodes a fully-instantiated type has. + max_ty_size: u64, + // Maximum depth (in terms of number of nodes) a fully-instantiated type has. + max_ty_depth: u64, } impl TypeBuilder { - pub const LEGACY_MAX_TYPE_INSTANTIATION_NODES: u64 = 128; - const LEGACY_TYPE_DEPTH_MAX: u64 = 256; - - pub fn is_legacy(&self) -> bool { - matches!(self, Self::Legacy) - } - pub fn with_limits(max_ty_size: u64, max_ty_depth: u64) -> Self { - Self::WithLimits { + Self { max_ty_size, max_ty_depth, } } - pub fn legacy() -> Self { - Self::Legacy - } - #[inline] pub fn create_bool_ty(&self) -> Type { Type::Bool @@ -763,58 +746,45 @@ impl TypeBuilder { /// Returns an error if the type size or depth are too large. #[inline] pub fn create_ref_ty(&self, inner_ty: &Type, is_mut: bool) -> PartialVMResult { - if self.is_legacy() { - let inner_ty = Box::new(inner_ty.clone()); - Ok(if is_mut { - Type::MutableReference(inner_ty) - } else { - Type::Reference(inner_ty) - }) + let mut count = 1; + let check = |c: &mut u64, d: u64| self.check(c, d); + let inner_ty = self + .clone_impl(inner_ty, &mut count, 2, check) + .map_err(|e| { + e.append_message_with_separator( + '.', + format!( + "Failed to create a (mutable: {}) reference type with inner type {}", + is_mut, inner_ty + ), + ) + })?; + let inner_ty = Box::new(inner_ty); + Ok(if is_mut { + Type::MutableReference(inner_ty) } else { - let mut count = 1; - let check = |c: &mut u64, d: u64| self.check(c, d); - let inner_ty = self - .clone_impl(inner_ty, &mut count, 2, check) - .map_err(|e| { - e.append_message_with_separator( - '.', - format!( - "Failed to create a (mutable: {}) reference type with inner type {}", - is_mut, inner_ty - ), - ) - })?; - let inner_ty = Box::new(inner_ty); - Ok(if is_mut { - Type::MutableReference(inner_ty) - } else { - Type::Reference(inner_ty) - }) - } + Type::Reference(inner_ty) + }) } /// Creates a vector type with the given element type, returning an error /// if the type size or depth are too large. #[inline] pub fn create_vec_ty(&self, elem_ty: &Type) -> PartialVMResult { - if self.is_legacy() { - Ok(Type::Vector(TriompheArc::new(elem_ty.clone()))) - } else { - let mut count = 1; - let check = |c: &mut u64, d: u64| self.check(c, d); - let elem_ty = self - .clone_impl(elem_ty, &mut count, 2, check) - .map_err(|e| { - e.append_message_with_separator( - '.', - format!( - "Failed to create a vector type with element type {}", - elem_ty - ), - ) - })?; - Ok(Type::Vector(TriompheArc::new(elem_ty))) - } + let mut count = 1; + let check = |c: &mut u64, d: u64| self.check(c, d); + let elem_ty = self + .clone_impl(elem_ty, &mut count, 2, check) + .map_err(|e| { + e.append_message_with_separator( + '.', + format!( + "Failed to create a vector type with element type {}", + elem_ty + ), + ) + })?; + Ok(Type::Vector(TriompheArc::new(elem_ty))) } #[inline] @@ -831,80 +801,37 @@ impl TypeBuilder { ty_params: &[Type], ty_args: &[Type], ) -> PartialVMResult { - if self.is_legacy() { - Ok(Type::StructInstantiation { - idx: struct_ty.idx, - ty_args: triomphe::Arc::new( - ty_params - .iter() - .map(|ty| self.create_ty_with_subst(ty, ty_args)) - .collect::>>()?, - ), - ability: AbilityInfo::generic_struct( - struct_ty.abilities, - struct_ty.phantom_ty_params_mask.clone(), - ), - }) - } else { - // We cannot call substitution API directly because we have to take into - // account struct type itself. We simply shift count and depth by 1 and - // call inner APIs, to save extra cloning. - let mut count = 1; - let check = |c: &mut u64, d: u64| self.check(c, d); - - let ty_args = ty_params - .iter() - .map(|ty| { - // Note that depth is 2 because we accounted for the parent struct type. - self.subst_impl(ty, ty_args, &mut count, 2, check) - .map_err(|e| { - e.append_message_with_separator( - '.', - format!( - "Failed to instantiate a type {} with type arguments {:?}", - ty, ty_args - ), - ) - }) - }) - .collect::>>()?; - - Ok(Type::StructInstantiation { - idx: struct_ty.idx, - ty_args: triomphe::Arc::new(ty_args), - ability: AbilityInfo::generic_struct( - struct_ty.abilities, - struct_ty.phantom_ty_params_mask.clone(), - ), - }) - } - } + // We cannot call substitution API directly because we have to take into + // account struct type itself. We simply shift count and depth by 1 and + // call inner APIs, to save extra cloning. + let mut count = 1; + let check = |c: &mut u64, d: u64| self.check(c, d); - // This has to be a separate method because there are two different legacy instantiation - // constructions, where one uses the legacy checks and the other does not. - pub fn create_struct_instantiation_ty_with_legacy_check( - &self, - struct_ty: &StructType, - ty_params: &[Type], - ty_args: &[Type], - ) -> PartialVMResult { - if self.is_legacy() { - Ok(Type::StructInstantiation { - idx: struct_ty.idx, - ty_args: triomphe::Arc::new( - ty_params - .iter() - .map(|ty| self.create_ty_with_subst_with_legacy_check(ty, ty_args)) - .collect::>()?, - ), - ability: AbilityInfo::generic_struct( - struct_ty.abilities, - struct_ty.phantom_ty_params_mask.clone(), - ), + let ty_args = ty_params + .iter() + .map(|ty| { + // Note that depth is 2 because we accounted for the parent struct type. + self.subst_impl(ty, ty_args, &mut count, 2, check) + .map_err(|e| { + e.append_message_with_separator( + '.', + format!( + "Failed to instantiate a type {} with type arguments {:?}", + ty, ty_args + ), + ) + }) }) - } else { - self.create_struct_instantiation_ty(struct_ty, ty_params, ty_args) - } + .collect::>>()?; + + Ok(Type::StructInstantiation { + idx: struct_ty.idx, + ty_args: triomphe::Arc::new(ty_args), + ability: AbilityInfo::generic_struct( + struct_ty.abilities, + struct_ty.phantom_ty_params_mask.clone(), + ), + }) } /// Creates a type for a Move constant. Note that constant types can be @@ -913,17 +840,13 @@ impl TypeBuilder { let mut count = 0; self.create_constant_ty_impl(const_tok, &mut count, 1) .map_err(|e| { - if self.is_legacy() { - e - } else { - e.append_message_with_separator( - '.', - format!( - "Failed to construct a type for constant token {:?}", - const_tok - ), - ) - } + e.append_message_with_separator( + '.', + format!( + "Failed to construct a type for constant token {:?}", + const_tok + ), + ) }) } @@ -939,96 +862,26 @@ impl TypeBuilder { /// Clones the given type, at the same time instantiating all its type parameters. pub fn create_ty_with_subst(&self, ty: &Type, ty_args: &[Type]) -> PartialVMResult { let mut count = 0; - - let check = |c: &mut u64, d: u64| { - if self.is_legacy() { - self.legacy_check_in_subst(c, d) - } else { - self.check(c, d) - } - }; - + let check = |c: &mut u64, d: u64| self.check(c, d); self.subst_impl(ty, ty_args, &mut count, 1, check) } - // This has to be a separate method because legacy type construction used both: - // 1) depth check only, - // 2) no checks - // in different places. When legacy implementation is removed, this function can - // be removed in favour of "normal" type substitution. - pub fn create_ty_with_subst_with_legacy_check( - &self, - ty: &Type, - ty_args: &[Type], - ) -> PartialVMResult { - if self.is_legacy() { - match ty { - Type::MutableReference(_) | Type::Reference(_) | Type::Vector(_) => { - if legacy_count_type_nodes(ty) > Self::LEGACY_MAX_TYPE_INSTANTIATION_NODES { - return Err(PartialVMError::new(StatusCode::TOO_MANY_TYPE_NODES)); - } - }, - Type::StructInstantiation { - ty_args: struct_inst, - .. - } => { - let mut sum_nodes = 1u64; - for ty in ty_args.iter().chain(struct_inst.iter()) { - sum_nodes = sum_nodes.saturating_add(legacy_count_type_nodes(ty)); - if sum_nodes > Self::LEGACY_MAX_TYPE_INSTANTIATION_NODES { - return Err(PartialVMError::new(StatusCode::TOO_MANY_TYPE_NODES)); - } - } - }, - Type::Address - | Type::Bool - | Type::Signer - | Type::Struct { .. } - | Type::TyParam(_) - | Type::U8 - | Type::U16 - | Type::U32 - | Type::U64 - | Type::U128 - | Type::U256 => (), - }; - } - self.create_ty_with_subst(ty, ty_args) - } - - fn legacy_check_in_subst(&self, _count: &mut u64, depth: u64) -> PartialVMResult<()> { - if depth > Self::LEGACY_TYPE_DEPTH_MAX { - return Err(PartialVMError::new(StatusCode::VM_MAX_TYPE_DEPTH_REACHED)); - } - Ok(()) - } - fn check(&self, count: &mut u64, depth: u64) -> PartialVMResult<()> { - match self { - Self::WithLimits { - max_ty_size, - max_ty_depth, - } => { - if *count >= *max_ty_size { - return Err( - PartialVMError::new(StatusCode::TOO_MANY_TYPE_NODES).with_message(format!( - "Type size is larger than maximum {}", - *max_ty_size - )), - ); - } - if depth > *max_ty_depth { - return Err(PartialVMError::new(StatusCode::VM_MAX_TYPE_DEPTH_REACHED) - .with_message(format!( - "Type depth is larger than maximum {}", - *max_ty_depth - ))); - } - }, - - // No checks in legacy implementation. Depth check for substitution - // is handled separately. - Self::Legacy => {}, + if *count >= self.max_ty_size { + return Err( + PartialVMError::new(StatusCode::TOO_MANY_TYPE_NODES).with_message(format!( + "Type size is larger than maximum {}", + self.max_ty_size + )), + ); + } + if depth > self.max_ty_depth { + return Err( + PartialVMError::new(StatusCode::VM_MAX_TYPE_DEPTH_REACHED).with_message(format!( + "Type depth is larger than maximum {}", + self.max_ty_depth + )), + ); } Ok(()) } @@ -1059,29 +912,19 @@ impl TypeBuilder { }, S::Struct(_) | S::StructInstantiation(_, _) => { - let msg = if self.is_legacy() { - "Unable to load const type signature".to_string() - } else { - "Struct constants are not supported".to_string() - }; return Err( PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) - .with_message(msg), + .with_message("Struct constants are not supported".to_string()), ); }, tok => { - let msg = if self.is_legacy() { - "Unable to load const type signature".to_string() - } else { - format!( - "{:?} is not allowed or is not a meaningful token for a constant", - tok - ) - }; return Err( PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) - .with_message(msg), + .with_message(format!( + "{:?} is not allowed or is not a meaningful token for a constant", + tok + )), ); }, }) @@ -1102,25 +945,14 @@ impl TypeBuilder { ty, |idx, c, d| match ty_args.get(idx as usize) { Some(ty) => self.clone_impl(ty, c, d, check), - None => { - let msg = if self.is_legacy() { - format!( - "type substitution failed: index out of bounds -- len {} got {}", - ty_args.len(), - idx - ) - } else { - format!( - "Type substitution failed: index {} is out of bounds for {} type arguments", - idx, - ty_args.len() - ) - }; - Err( - PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) - .with_message(msg), - ) - }, + None => Err( + PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) + .with_message(format!( + "Type substitution failed: index {} is out of bounds for {} type arguments", + idx, + ty_args.len() + )), + ), }, count, depth, @@ -1293,31 +1125,6 @@ impl TypeBuilder { } } -pub fn legacy_count_type_nodes(ty: &Type) -> u64 { - let mut todo = vec![ty]; - let mut result = 0; - while let Some(ty) = todo.pop() { - match ty { - Type::Vector(ty) => { - result += 1; - todo.push(ty); - }, - Type::Reference(ty) | Type::MutableReference(ty) => { - result += 1; - todo.push(ty); - }, - Type::StructInstantiation { ty_args, .. } => { - result += 1; - todo.extend(ty_args.iter()) - }, - _ => { - result += 1; - }, - } - } - result -} - #[cfg(test)] mod unit_tests { use super::*; @@ -1465,28 +1272,14 @@ mod unit_tests { // Limits are irrelevant here. let ty_builder = TypeBuilder::with_limits(1, 1); - let legacy_ty_builder = TypeBuilder::legacy(); assert_eq!(ty_builder.create_u8_ty(), U8); - assert_eq!(legacy_ty_builder.create_u8_ty(), U8); - assert_eq!(ty_builder.create_u16_ty(), U16); - assert_eq!(legacy_ty_builder.create_u16_ty(), U16); - assert_eq!(ty_builder.create_u32_ty(), U32); - assert_eq!(legacy_ty_builder.create_u32_ty(), U32); - assert_eq!(ty_builder.create_u64_ty(), U64); - assert_eq!(legacy_ty_builder.create_u64_ty(), U64); - assert_eq!(ty_builder.create_u128_ty(), U128); - assert_eq!(legacy_ty_builder.create_u128_ty(), U128); - assert_eq!(ty_builder.create_u256_ty(), U256); - assert_eq!(legacy_ty_builder.create_u256_ty(), U256); - assert_eq!(ty_builder.create_bool_ty(), Bool); - assert_eq!(legacy_ty_builder.create_bool_ty(), Bool); } #[test] @@ -1496,9 +1289,6 @@ mod unit_tests { // Limits are not relevant here. let struct_ty = TypeBuilder::with_limits(1, 1).create_struct_ty(idx, ability_info.clone()); - let legacy_struct_ty = TypeBuilder::legacy().create_struct_ty(idx, ability_info); - - assert_eq!(&struct_ty, &legacy_struct_ty); assert_matches!(struct_ty, Type::Struct { .. }); } @@ -1512,12 +1302,7 @@ mod unit_tests { // Should succeed, type size limit is 5, and we have 5 nodes. let ty_builder = TypeBuilder::with_limits(5, 100); let ty_args = [Bool, Vector(TriompheArc::new(Bool))]; - let ty = - assert_ok!(ty_builder.create_struct_instantiation_ty(&struct_ty, &ty_params, &ty_args)); - let legacy_ty = - assert_ok!(TypeBuilder::legacy() - .create_struct_instantiation_ty(&struct_ty, &ty_params, &ty_args)); - assert_eq!(ty, legacy_ty); + assert_ok!(ty_builder.create_struct_instantiation_ty(&struct_ty, &ty_params, &ty_args)); // Should fail, we have size of 6 now. let ty_args = [ @@ -1533,12 +1318,7 @@ mod unit_tests { let nested_vec = Vector(TriompheArc::new(Vector(TriompheArc::new(Bool)))); let ty_args = vec![Bool, nested_vec.clone()]; let ty_builder = TypeBuilder::with_limits(100, 4); - let ty = - assert_ok!(ty_builder.create_struct_instantiation_ty(&struct_ty, &ty_params, &ty_args)); - let legacy_ty = - assert_ok!(TypeBuilder::legacy() - .create_struct_instantiation_ty(&struct_ty, &ty_params, &ty_args)); - assert_eq!(ty, legacy_ty); + assert_ok!(ty_builder.create_struct_instantiation_ty(&struct_ty, &ty_params, &ty_args)); // Should fail, we have depth of 5 now. let ty_params = vec![Bool, Vector(TriompheArc::new(nested_vec))]; @@ -1552,26 +1332,19 @@ mod unit_tests { fn test_create_vec_ty() { let max_ty_depth = 5; let ty_builder = TypeBuilder::with_limits(100, max_ty_depth); - let legacy_ty_builder = TypeBuilder::legacy(); - // Make sure the new type builder creates vector types in the same way as the legacy once. let mut depth = 1; let mut ty = Type::Bool; while depth < max_ty_depth { - let legacy_ty = assert_ok!(legacy_ty_builder.create_vec_ty(&ty)); ty = assert_ok!(ty_builder.create_vec_ty(&ty)); - - assert_eq!(&ty, &legacy_ty); assert_matches!(ty, Type::Vector(_)); depth += 1; } assert_eq!(depth, max_ty_depth); - // New type builder fails on exceeding the depth, but not the legacy one. + // Type creation fails on exceeding the depth. let err = assert_err!(ty_builder.create_vec_ty(&ty)); assert_eq!(err.major_status(), StatusCode::VM_MAX_TYPE_DEPTH_REACHED); - let legacy_ty = assert_ok!(legacy_ty_builder.create_vec_ty(&ty)); - assert_matches!(legacy_ty, Type::Vector(_)); // The checks are always ordered: first number of nodes, then depth. Using // a type builder with smaller than depth size limit must return a different @@ -1586,15 +1359,11 @@ mod unit_tests { fn test_create_ref_ty() { let max_ty_depth = 5; let ty_builder = TypeBuilder::with_limits(100, max_ty_depth); - let legacy_ty_builder = TypeBuilder::legacy(); let mut depth = 1; let mut ty = Type::Bool; while depth < max_ty_depth { - let legacy_ty = assert_ok!(legacy_ty_builder.create_ref_ty(&ty, false)); ty = assert_ok!(ty_builder.create_ref_ty(&ty, false)); - - assert_eq!(&ty, &legacy_ty); assert_matches!(ty, Type::Reference(_)); depth += 1; } @@ -1602,8 +1371,6 @@ mod unit_tests { let err = assert_err!(ty_builder.create_ref_ty(&ty, false)); assert_eq!(err.major_status(), StatusCode::VM_MAX_TYPE_DEPTH_REACHED); - let legacy_ty = assert_ok!(legacy_ty_builder.create_ref_ty(&ty, false)); - assert_matches!(legacy_ty, Type::Reference(_)); let max_ty_size = 5; let ty_builder = TypeBuilder::with_limits(max_ty_size, 100); @@ -1615,15 +1382,11 @@ mod unit_tests { fn test_create_mut_ref_ty() { let max_ty_depth = 5; let ty_builder = TypeBuilder::with_limits(100, max_ty_depth); - let legacy_ty_builder = TypeBuilder::legacy(); let mut depth = 1; let mut ty = Type::Bool; while depth < max_ty_depth { - let legacy_ty = assert_ok!(legacy_ty_builder.create_ref_ty(&ty, true)); ty = assert_ok!(ty_builder.create_ref_ty(&ty, true)); - - assert_eq!(&ty, &legacy_ty); assert_matches!(ty, Type::MutableReference(_)); depth += 1; } @@ -1631,8 +1394,6 @@ mod unit_tests { let err = assert_err!(ty_builder.create_ref_ty(&ty, true)); assert_eq!(err.major_status(), StatusCode::VM_MAX_TYPE_DEPTH_REACHED); - let legacy_ty = assert_ok!(legacy_ty_builder.create_ref_ty(&ty, true)); - assert_matches!(legacy_ty, Type::MutableReference(_)); let max_ty_size = 5; let ty_builder = TypeBuilder::with_limits(max_ty_size, 100); @@ -1647,55 +1408,18 @@ mod unit_tests { let max_ty_depth = 5; let ty_builder = TypeBuilder::with_limits(100, max_ty_depth); - let legacy_ty_builder = TypeBuilder::legacy(); assert_eq!(assert_ok!(ty_builder.create_constant_ty(&S::U8)), U8); - assert_eq!(assert_ok!(legacy_ty_builder.create_constant_ty(&S::U8)), U8); - assert_eq!(assert_ok!(ty_builder.create_constant_ty(&S::U16)), U16); - assert_eq!( - assert_ok!(legacy_ty_builder.create_constant_ty(&S::U16)), - U16 - ); - assert_eq!(assert_ok!(ty_builder.create_constant_ty(&S::U32)), U32); - assert_eq!( - assert_ok!(legacy_ty_builder.create_constant_ty(&S::U32)), - U32 - ); - assert_eq!(assert_ok!(ty_builder.create_constant_ty(&S::U64)), U64); - assert_eq!( - assert_ok!(legacy_ty_builder.create_constant_ty(&S::U64)), - U64 - ); - assert_eq!(assert_ok!(ty_builder.create_constant_ty(&S::U128)), U128); - assert_eq!( - assert_ok!(legacy_ty_builder.create_constant_ty(&S::U128)), - U128 - ); - assert_eq!(assert_ok!(ty_builder.create_constant_ty(&S::U256)), U256); - assert_eq!( - assert_ok!(legacy_ty_builder.create_constant_ty(&S::U256)), - U256 - ); - assert_eq!(assert_ok!(ty_builder.create_constant_ty(&S::Bool)), Bool); - assert_eq!( - assert_ok!(legacy_ty_builder.create_constant_ty(&S::Bool)), - Bool - ); - assert_eq!( assert_ok!(ty_builder.create_constant_ty(&S::Address)), Address ); - assert_eq!( - assert_ok!(legacy_ty_builder.create_constant_ty(&S::Address)), - Address - ); // Vectors are special, because we limit their depth (and size). // Here, we test the boundary cases. @@ -1703,18 +1427,12 @@ mod unit_tests { for depth in [max_ty_depth - 1, max_ty_depth] { let (expected_ty, vec_tok, _) = nested_vec_for_test(depth); let ty = assert_ok!(ty_builder.create_constant_ty(&vec_tok)); - let legacy_ty = assert_ok!(legacy_ty_builder.create_constant_ty(&vec_tok)); - assert_eq!(&ty, &legacy_ty); assert_eq!(&ty, &expected_ty); } - let (expected_legacy_ty, vec_tok, _) = nested_vec_for_test(max_ty_depth + 1); + let (_, vec_tok, _) = nested_vec_for_test(max_ty_depth + 1); let err = assert_err!(ty_builder.create_constant_ty(&vec_tok)); assert_eq!(err.major_status(), StatusCode::VM_MAX_TYPE_DEPTH_REACHED); - assert_eq!( - assert_ok!(legacy_ty_builder.create_constant_ty(&vec_tok)), - expected_legacy_ty - ); let max_ty_size = 5; let ty_builder = TypeBuilder::with_limits(max_ty_size, 100); @@ -1722,8 +1440,6 @@ mod unit_tests { for size in [max_ty_size - 1, max_ty_size] { let (expected_ty, vec_tok, _) = nested_vec_for_test(size); let ty = assert_ok!(ty_builder.create_constant_ty(&vec_tok)); - let legacy_ty = assert_ok!(legacy_ty_builder.create_constant_ty(&vec_tok)); - assert_eq!(&ty, &legacy_ty); assert_eq!(&ty, &expected_ty); } @@ -1735,26 +1451,20 @@ mod unit_tests { let struct_tok = S::Struct(StructHandleIndex::new(0)); assert_err!(ty_builder.create_constant_ty(&struct_tok)); - assert_err!(legacy_ty_builder.create_constant_ty(&struct_tok)); let struct_instantiation_tok = S::StructInstantiation(StructHandleIndex::new(0), vec![]); assert_err!(ty_builder.create_constant_ty(&struct_instantiation_tok)); - assert_err!(legacy_ty_builder.create_constant_ty(&struct_instantiation_tok)); assert_err!(ty_builder.create_constant_ty(&S::Signer)); - assert_err!(legacy_ty_builder.create_constant_ty(&S::Signer)); let ref_tok = S::Reference(Box::new(S::U8)); assert_err!(ty_builder.create_constant_ty(&ref_tok)); - assert_err!(legacy_ty_builder.create_constant_ty(&ref_tok)); let mut_ref_tok = S::Reference(Box::new(S::U8)); assert_err!(ty_builder.create_constant_ty(&mut_ref_tok)); - assert_err!(legacy_ty_builder.create_constant_ty(&mut_ref_tok)); let ty_param_tok = S::TypeParameter(0); assert_err!(ty_builder.create_constant_ty(&ty_param_tok)); - assert_err!(legacy_ty_builder.create_constant_ty(&ty_param_tok)); } #[test] @@ -1765,74 +1475,35 @@ mod unit_tests { let max_ty_size = 11; let max_ty_depth = 5; let ty_builder = TypeBuilder::with_limits(max_ty_size, max_ty_depth); - let legacy_ty_builder = TypeBuilder::legacy(); let no_op = |_: &StructTag| unreachable!("Should not be called"); // Primitive types. assert_eq!(assert_ok!(ty_builder.create_ty(&T::U8, no_op)), U8); - assert_eq!(assert_ok!(legacy_ty_builder.create_ty(&T::U8, no_op)), U8); - assert_eq!(assert_ok!(ty_builder.create_ty(&T::U16, no_op)), U16); - assert_eq!(assert_ok!(legacy_ty_builder.create_ty(&T::U16, no_op)), U16); - assert_eq!(assert_ok!(ty_builder.create_ty(&T::U32, no_op)), U32); - assert_eq!(assert_ok!(legacy_ty_builder.create_ty(&T::U32, no_op)), U32); - assert_eq!(assert_ok!(ty_builder.create_ty(&T::U64, no_op)), U64); - assert_eq!(assert_ok!(legacy_ty_builder.create_ty(&T::U64, no_op)), U64); - assert_eq!(assert_ok!(ty_builder.create_ty(&T::U128, no_op)), U128); - assert_eq!( - assert_ok!(legacy_ty_builder.create_ty(&T::U128, no_op)), - U128 - ); - assert_eq!(assert_ok!(ty_builder.create_ty(&T::U256, no_op)), U256); - assert_eq!( - assert_ok!(legacy_ty_builder.create_ty(&T::U256, no_op)), - U256 - ); - assert_eq!(assert_ok!(ty_builder.create_ty(&T::Bool, no_op)), Bool); - assert_eq!( - assert_ok!(legacy_ty_builder.create_ty(&T::Bool, no_op)), - Bool - ); - assert_eq!( assert_ok!(ty_builder.create_ty(&T::Address, no_op)), Address ); - assert_eq!( - assert_ok!(legacy_ty_builder.create_ty(&T::Address, no_op)), - Address - ); - assert_eq!(assert_ok!(ty_builder.create_ty(&T::Signer, no_op)), Signer); - assert_eq!( - assert_ok!(legacy_ty_builder.create_ty(&T::Signer, no_op)), - Signer - ); // Vectors. for depth in [max_ty_depth - 1, max_ty_depth] { let (expected_ty, _, vec_tag) = nested_vec_for_test(depth); let ty = assert_ok!(ty_builder.create_ty(&vec_tag, no_op)); - let legacy_ty = assert_ok!(legacy_ty_builder.create_ty(&vec_tag, no_op)); - assert_eq!(&ty, &legacy_ty); assert_eq!(&ty, &expected_ty); } - let (expected_legacy_ty, _, vec_tag) = nested_vec_for_test(max_ty_depth + 1); + let (_, _, vec_tag) = nested_vec_for_test(max_ty_depth + 1); let err = assert_err!(ty_builder.create_ty(&vec_tag, no_op)); assert_eq!(err.major_status(), StatusCode::VM_MAX_TYPE_DEPTH_REACHED); - assert_eq!( - assert_ok!(legacy_ty_builder.create_ty(&vec_tag, no_op)), - expected_legacy_ty - ); let max_ty_size = 5; let ty_builder = TypeBuilder::with_limits(max_ty_size, 100); @@ -1840,8 +1511,6 @@ mod unit_tests { for size in [max_ty_size - 1, max_ty_size] { let (expected_ty, _, vec_tag) = nested_vec_for_test(size); let ty = assert_ok!(ty_builder.create_ty(&vec_tag, no_op)); - let legacy_ty = assert_ok!(legacy_ty_builder.create_ty(&vec_tag, no_op)); - assert_eq!(&ty, &legacy_ty); assert_eq!(&ty, &expected_ty); } diff --git a/types/src/on_chain_config/aptos_features.rs b/types/src/on_chain_config/aptos_features.rs index aa6be6517d748..8a04eeb2ae6c8 100644 --- a/types/src/on_chain_config/aptos_features.rs +++ b/types/src/on_chain_config/aptos_features.rs @@ -287,10 +287,6 @@ impl Features { self.is_enabled(FeatureFlag::REFUNDABLE_BYTES) } - pub fn is_limit_type_size_enabled(&self) -> bool { - self.is_enabled(FeatureFlag::LIMIT_VM_TYPE_SIZE) - } - pub fn is_abort_if_multisig_payload_mismatch_enabled(&self) -> bool { self.is_enabled(FeatureFlag::ABORT_IF_MULTISIG_PAYLOAD_MISMATCH) }