Skip to content

Commit

Permalink
[move] Remove legacy type builder (#14002)
Browse files Browse the repository at this point in the history
  • Loading branch information
georgemitenkov authored Jul 17, 2024
1 parent 7228496 commit 0f49e1c
Show file tree
Hide file tree
Showing 10 changed files with 144 additions and 565 deletions.
23 changes: 9 additions & 14 deletions aptos-move/aptos-vm-types/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}

Expand All @@ -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,
Expand Down
7 changes: 3 additions & 4 deletions aptos-move/aptos-vm/src/move_vm_ext/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -113,16 +113,15 @@ 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(),
ty_builder,
)
},
Err(_) => {
let ty_builder = aptos_default_ty_builder(env.features());
let ty_builder = aptos_default_ty_builder();
(
NativeGasParameters::zeros(),
MiscGasParameters::zeros(),
Expand Down
14 changes: 2 additions & 12 deletions aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -115,15 +111,13 @@ 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)
.expect("Module must publish");

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(),
Expand All @@ -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::<Vec<u8>>::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
);
}
2 changes: 1 addition & 1 deletion third_party/move/move-vm/runtime/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down
13 changes: 4 additions & 9 deletions third_party/move/move-vm/runtime/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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::<PartialVMResult<Vec<_>>>()?
}
} else {
Expand Down Expand Up @@ -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)?;
Expand Down Expand Up @@ -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)?;
}
}
Expand Down
67 changes: 7 additions & 60 deletions third_party/move/move-vm/runtime/src/loader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -325,20 +325,6 @@ impl Loader {
.map(|ty| self.load_type(ty, data_store, module_store))
.collect::<VMResult<Vec<_>>>()?;

#[allow(clippy::collapsible_if)]
if self.ty_builder().is_legacy() {
if ty_args.iter().map(legacy_count_type_nodes).sum::<u64>()
> 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 {}",
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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())
}
Expand Down Expand Up @@ -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::<PartialVMResult<Vec<_>>>()?;
let (mut field_layouts, field_has_identifier_mappings): (Vec<MoveTypeLayout>, Vec<bool>) =
field_tys
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)],
}
Loading

0 comments on commit 0f49e1c

Please sign in to comment.