diff --git a/crates/rooch-framework-tests/tests/cases/mvir_tests/data_struct_func/data_struct_func_invalid_move_std_option_option_as_struct.exp b/crates/rooch-framework-tests/tests/cases/mvir_tests/data_struct_func/data_struct_func_invalid_move_std_option_option_as_struct.exp deleted file mode 100644 index 5916752906..0000000000 --- a/crates/rooch-framework-tests/tests/cases/mvir_tests/data_struct_func/data_struct_func_invalid_move_std_option_option_as_struct.exp +++ /dev/null @@ -1,4 +0,0 @@ -processed 1 task - -task 0 'publish'. lines 1-39: -status ABORTED with code 10009 in 0000000000000000000000000000000000000000000000000000000000000002::move_module diff --git a/crates/rooch-framework-tests/tests/cases/mvir_tests/data_struct_func/data_struct_func_valid_move_std_option_option_as_struct.exp b/crates/rooch-framework-tests/tests/cases/mvir_tests/data_struct_func/data_struct_func_valid_move_std_option_option_as_struct.exp new file mode 100644 index 0000000000..a9c1355ceb --- /dev/null +++ b/crates/rooch-framework-tests/tests/cases/mvir_tests/data_struct_func/data_struct_func_valid_move_std_option_option_as_struct.exp @@ -0,0 +1,4 @@ +processed 1 task + +task 0 'publish'. lines 1-39: +status EXECUTED diff --git a/crates/rooch-framework-tests/tests/cases/mvir_tests/data_struct_func/data_struct_func_invalid_move_std_option_option_as_struct.mvir b/crates/rooch-framework-tests/tests/cases/mvir_tests/data_struct_func/data_struct_func_valid_move_std_option_option_as_struct.mvir similarity index 100% rename from crates/rooch-framework-tests/tests/cases/mvir_tests/data_struct_func/data_struct_func_invalid_move_std_option_option_as_struct.mvir rename to crates/rooch-framework-tests/tests/cases/mvir_tests/data_struct_func/data_struct_func_valid_move_std_option_option_as_struct.mvir diff --git a/moveos/moveos-verifier/src/build.rs b/moveos/moveos-verifier/src/build.rs index e47fae4fde..9ccc1de581 100644 --- a/moveos/moveos-verifier/src/build.rs +++ b/moveos/moveos-verifier/src/build.rs @@ -1,7 +1,10 @@ // Copyright (c) RoochNetwork // SPDX-License-Identifier: Apache-2.0 -use crate::metadata::{run_extended_checks, RuntimeModuleMetadataV1}; +use crate::metadata::{ + run_extended_checks, RuntimeModuleMetadataV1, DATA_STRUCT_ATTRIBUTE, + DATA_STRUCT_FUNC_ATTRIBUTE, PRIVATE_GENERICS_ATTRIBUTE, +}; use codespan_reporting::diagnostic::Severity; use itertools::Itertools; use move_binary_format::CompiledModule; @@ -396,7 +399,7 @@ pub fn compile_and_inject_metadata( let mut rooch_metadata = RuntimeModuleMetadataV1::default(); for (metadata_type, metadata_item) in ast_metadata.value { - if metadata_type == "private_generics" { + if metadata_type == PRIVATE_GENERICS_ATTRIBUTE { let mut private_generics_map: BTreeMap> = BTreeMap::new(); for (metadata_key, metadata_value) in metadata_item.iter() { let mut generic_type_indices: Vec = Vec::new(); @@ -414,7 +417,7 @@ pub fn compile_and_inject_metadata( rooch_metadata.private_generics_indices = private_generics_map; } - if metadata_type == "data_struct" { + if metadata_type == DATA_STRUCT_ATTRIBUTE { let mut data_structs_map: BTreeMap = BTreeMap::new(); for (metadata_key, metadata_value) in metadata_item.iter() { for idx_expr in metadata_value.iter() { @@ -430,7 +433,7 @@ pub fn compile_and_inject_metadata( rooch_metadata.data_struct_map = data_structs_map; } - if metadata_type == "data_struct_func" { + if metadata_type == DATA_STRUCT_FUNC_ATTRIBUTE { let mut data_struct_func_map: BTreeMap> = BTreeMap::new(); for (metadata_key, metadata_value) in metadata_item.iter() { let mut generic_type_indices: Vec = Vec::new(); diff --git a/moveos/moveos-verifier/src/error_code.rs b/moveos/moveos-verifier/src/error_code.rs index a33e32ad9c..72445a1f64 100644 --- a/moveos/moveos-verifier/src/error_code.rs +++ b/moveos/moveos-verifier/src/error_code.rs @@ -62,6 +62,8 @@ pub enum ErrorCode { INVALID_DATA_STRUCT_WITHOUT_COPY_ABILITY = 10013, INVALID_DATA_STRUCT_NOT_ALLOWED_TYPE = 10014, INVALID_DATA_STRUCT_NOT_IN_MODULE_METADATA = 10015, + INVALID_DATA_STRUCT_WITH_TYPE_PARAMETER = 10016, + INVALID_DATA_STRUCT_OPTION_WITHOUT_TYPE_PARAMETER = 10017, INVALID_ENTRY_FUNC_SIGNATURE = 11000, INVALID_PARAM_TYPE_ENTRY_FUNCTION = 11001, diff --git a/moveos/moveos-verifier/src/metadata.rs b/moveos/moveos-verifier/src/metadata.rs index 43235ca703..b6c2876a97 100644 --- a/moveos/moveos-verifier/src/metadata.rs +++ b/moveos/moveos-verifier/src/metadata.rs @@ -42,13 +42,15 @@ pub static mut GLOBAL_DATA_STRUCT_FUNC: GlobalVariableWithRWLocker>> = Lazy::new(|| BTreeMap::new()); -const PRIVATE_GENERICS_ATTRIBUTE: &str = "private_generics"; +pub const PRIVATE_GENERICS_ATTRIBUTE: &str = "private_generics"; const GAS_FREE_ATTRIBUTE: &str = "gas_free"; const GAS_FREE_VALIDATE: &str = "gas_validate"; const GAS_FREE_CHARGE_POST: &str = "gas_charge_post"; -const DATA_STRUCT_ATTRIBUTE: &str = "data_struct"; +pub const DATA_STRUCT_ATTRIBUTE: &str = "data_struct"; + +pub const DATA_STRUCT_FUNC_ATTRIBUTE: &str = "data_struct_func"; /// Enumeration of potentially known attributes #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] diff --git a/moveos/moveos-verifier/src/verifier.rs b/moveos/moveos-verifier/src/verifier.rs index da57983af5..5873814e31 100644 --- a/moveos/moveos-verifier/src/verifier.rs +++ b/moveos/moveos-verifier/src/verifier.rs @@ -449,7 +449,7 @@ where type_parameters, } = view.function_instantiation_at(finst_idx); - let full_path_func_name = build_full_function_name(handle, view); + let full_path_func_name = build_full_function_name(handle, &view); let type_arguments = &view.signature_at(*type_parameters).0; let private_generics_types = @@ -510,7 +510,7 @@ where Ok(true) } -fn build_full_function_name(fhandle_idx: &FunctionHandleIndex, view: BinaryIndexedView) -> String { +fn build_full_function_name(fhandle_idx: &FunctionHandleIndex, view: &BinaryIndexedView) -> String { let fhandle = view.function_handle_at(*fhandle_idx); let module_handle = view.module_handle_at(fhandle.module); @@ -875,106 +875,24 @@ where Some(v) => v, }; - match type_arg { - SignatureToken::Struct(struct_handle_idx) => { - let struct_handle = - view.struct_handle_at(*struct_handle_idx); - let module_handle = - view.module_handle_at(struct_handle.module); - let module_name = format!( - "{}::{}", - view.address_identifier_at(module_handle.address) - .to_hex_literal(), - view.identifier_at(module_handle.name), - ); - - // load module from struct handle - let compiled_module_opt = - load_compiled_module_from_struct_handle( - db, - &view, - *struct_handle_idx, - verified_modules, - ); - if let Some(callee_module) = compiled_module_opt { - if let Err(err) = - check_metadata_format(&callee_module) - { - return Err(PartialVMError::new( - StatusCode::ABORTED, - ) - .with_message(err.to_string()) - .with_sub_status( - ErrorCode::MALFORMED_METADATA.into(), - ) - .finish(Location::Module( - callee_module.self_id(), - ))); - } - - // Find the definition records of compile-time data_struct from CompiledModule. - let metadata_opt = - get_metadata_from_compiled_module( - &callee_module, - ); - if let Some(metadata) = metadata_opt { - let _ = metadata - .data_struct_map - .iter() - .map(|(key, value)| { - data_structs_map - .insert(key.clone(), *value); - }) - .collect::>(); - } - } - - let full_struct_name = format!( - "{}::{}", - module_name, - view.identifier_at(struct_handle.name) - ); - // allow string::String, ascii::String as data struct - if is_allowed_data_struct_type( - full_struct_name.as_str(), - ) { - continue; - } - - let is_data_struct_opt = - data_structs_map.get(full_struct_name.as_str()); - if is_data_struct_opt.is_none() { - let caller_func_name = - build_full_function_name(&func.function, view); - let error_msg = format!("function {:} call {:} with type {:} is not a data struct.", - caller_func_name, full_path_func_name, full_struct_name); - return generate_vm_error( - ErrorCode::INVALID_DATA_STRUCT, - error_msg, - Some(*fhandle_idx), - caller_module, - ); - } - } - SignatureToken::Address => {} - SignatureToken::Bool => {} - SignatureToken::U8 => {} - SignatureToken::U16 => {} - SignatureToken::U32 => {} - SignatureToken::U64 => {} - SignatureToken::U128 => {} - SignatureToken::U256 => {} - _ => { - let error_msg = format!("The type parameter when calling function {} is now allowed", - full_path_func_name); - return generate_vm_error( - ErrorCode::INVALID_DATA_STRUCT_TYPE, - error_msg, - Some(*fhandle_idx), - caller_module, - ); - } - } + let data_structs_map_in_struct_module = + load_data_structs_map_from_struct( + type_arg, + db, + &view, + verified_modules, + )?; + data_structs_map.extend(data_structs_map_in_struct_module); + + check_field_type( + type_arg, + &view, + &full_path_func_name, + data_structs_map.clone(), + *fhandle_idx, + func.clone(), + caller_module, + )?; } } } @@ -988,6 +906,189 @@ where } } +fn load_data_structs_map_from_struct( + type_arg: &SignatureToken, + db: &Resolver, + view: &BinaryIndexedView, + verified_modules: &mut BTreeMap, +) -> VMResult> +where + Resolver: ModuleResolver, +{ + match type_arg { + SignatureToken::Struct(struct_handle_idx) => { + load_data_structs(db, view, *struct_handle_idx, verified_modules) + } + SignatureToken::StructInstantiation(struct_handle_idx, _) => { + load_data_structs(db, view, *struct_handle_idx, verified_modules) + } + _ => Ok(BTreeMap::new()), + } +} + +fn load_data_structs( + db: &Resolver, + view: &BinaryIndexedView, + struct_handle_idx: StructHandleIndex, + verified_modules: &mut BTreeMap, +) -> VMResult> +where + Resolver: ModuleResolver, +{ + let mut data_structs_map: BTreeMap = BTreeMap::new(); + + // load module from struct handle + let compiled_module_opt = + load_compiled_module_from_struct_handle(db, view, struct_handle_idx, verified_modules); + + if let Some(callee_module) = compiled_module_opt { + if let Err(err) = check_metadata_format(&callee_module) { + return Err(PartialVMError::new(StatusCode::ABORTED) + .with_message(err.to_string()) + .with_sub_status(ErrorCode::MALFORMED_METADATA.into()) + .finish(Location::Module(callee_module.self_id()))); + } + + // Find the definition records of compile-time data_struct from CompiledModule. + let metadata_opt = get_metadata_from_compiled_module(&callee_module); + if let Some(metadata) = metadata_opt { + let _ = metadata + .data_struct_map + .iter() + .map(|(key, value)| { + data_structs_map.insert(key.clone(), *value); + }) + .collect::>(); + } + Ok(data_structs_map) + } else { + Ok(data_structs_map) + } +} + +fn check_field_type( + type_arg: &SignatureToken, + view: &BinaryIndexedView, + full_path_func_name: &String, + data_structs_map: BTreeMap, + fhandle_idx: FunctionHandleIndex, + func: FunctionDefinition, + caller_module: &CompiledModule, +) -> VMResult { + match type_arg { + SignatureToken::Struct(struct_handle_idx) => { + let struct_handle = view.struct_handle_at(*struct_handle_idx); + let module_handle = view.module_handle_at(struct_handle.module); + let module_name = format!( + "{}::{}", + view.address_identifier_at(module_handle.address) + .to_hex_literal(), + view.identifier_at(module_handle.name), + ); + + let full_struct_name = format!( + "{}::{}", + module_name, + view.identifier_at(struct_handle.name) + ); + // allow string::String, ascii::String as data struct + if is_allowed_data_struct_type(full_struct_name.as_str()) { + return Ok(true); + } + + let is_data_struct_opt = data_structs_map.get(full_struct_name.as_str()); + if is_data_struct_opt.is_none() { + let caller_func_name = build_full_function_name(&func.function, view); + let error_msg = format!( + "function {:} call {:} with type {:} is not a data struct.", + caller_func_name, full_path_func_name, full_struct_name + ); + return generate_vm_error( + ErrorCode::INVALID_DATA_STRUCT, + error_msg, + Some(fhandle_idx), + caller_module, + ); + } + Ok(true) + } + SignatureToken::StructInstantiation(struct_handle_idx, type_parameters) => { + let struct_handle = view.struct_handle_at(*struct_handle_idx); + let module_handle = view.module_handle_at(struct_handle.module); + let module_name = format!( + "{}::{}", + view.address_identifier_at(module_handle.address) + .to_hex_literal(), + view.identifier_at(module_handle.name), + ); + + let full_struct_name = format!( + "{}::{}", + module_name, + view.identifier_at(struct_handle.name) + ); + + if is_std_option_type(&full_struct_name) { + let first_type_parameter_opt = type_parameters.first(); + match first_type_parameter_opt { + None => { + let error_msg = format!( + "The type parameter when calling function {} is missed", + full_path_func_name + ); + generate_vm_error( + ErrorCode::INVALID_DATA_STRUCT_OPTION_WITHOUT_TYPE_PARAMETER, + error_msg, + Some(fhandle_idx), + caller_module, + ) + } + Some(first_type_parameter) => check_field_type( + first_type_parameter, + view, + full_path_func_name, + data_structs_map.clone(), + fhandle_idx, + func.clone(), + caller_module, + ), + } + } else { + let error_msg = format!( + "The type parameter when calling function {} is now allowed", + full_path_func_name + ); + generate_vm_error( + ErrorCode::INVALID_DATA_STRUCT_WITH_TYPE_PARAMETER, + error_msg, + Some(fhandle_idx), + caller_module, + ) + } + } + SignatureToken::Address => Ok(true), + SignatureToken::Bool => Ok(true), + SignatureToken::U8 => Ok(true), + SignatureToken::U16 => Ok(true), + SignatureToken::U32 => Ok(true), + SignatureToken::U64 => Ok(true), + SignatureToken::U128 => Ok(true), + SignatureToken::U256 => Ok(true), + _ => { + let error_msg = format!( + "The type parameter when calling function {} is now allowed", + full_path_func_name + ); + generate_vm_error( + ErrorCode::INVALID_DATA_STRUCT_TYPE, + error_msg, + Some(fhandle_idx), + caller_module, + ) + } + } +} + fn generate_full_module_name( fhandle_index: FunctionHandleIndex, view: BinaryIndexedView,