Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the data_struct validation in the verifier #1663

Merged
merged 3 commits into from
May 10, 2024
Merged

Refactor the data_struct validation in the verifier #1663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fbe5bde
use the data_struct that owned by the module itself
steelgeek091 May 9, 2024
7b342ca
remove println macros
steelgeek091 May 9, 2024
459e441
use the internal verified_modules
steelgeek091 May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/rooch/src/commands/move_cli/commands/publish.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ impl CommandAction<ExecuteTransactionResponseView> for Publish {
let sorted_modules = sort_by_dependency_order(modules.iter_modules())?;
let resolver = context.get_client().await?;
// Serialize and collect module binaries into bundles
verifier::verify_modules(&sorted_modules, &resolver)?;
for module in sorted_modules {
let module_address = module.self_id().address().to_owned();
if module_address != pkg_address {
Expand All @@ -101,7 +102,6 @@ impl CommandAction<ExecuteTransactionResponseView> for Publish {
pkg_address.clone(),
)));
};
verifier::verify_module(&module, &resolver)?;
let mut binary: Vec<u8> = vec![];
module.serialize(&mut binary)?;
bundles.push(binary);
Expand Down
13 changes: 12 additions & 1 deletion frameworks/moveos-stdlib/src/natives/moveos_stdlib/move_module.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,24 @@ fn native_sort_and_verify_modules_inner(
let module_context = context.extensions_mut().get_mut::<NativeModuleContext>();
let mut module_ids = vec![];
let mut init_identifier = vec![];

let verify_result =
moveos_verifier::verifier::verify_modules(&compiled_modules, module_context.resolver);
match verify_result {
Ok(_) => {}
Err(e) => {
log::info!("modules verification error: {:?}", e);
return Ok(NativeResult::err(cost, E_MODULE_VERIFICATION_ERROR));
}
}

for module in &compiled_modules {
let module_address = *module.self_id().address();

if module_address != account_address {
return Ok(NativeResult::err(cost, E_ADDRESS_NOT_MATCH_WITH_SIGNER));
}
let result = moveos_verifier::verifier::verify_module(module, module_context.resolver);
let result = moveos_verifier::verifier::verify_init_function(module);
match result {
Ok(res) => {
if res {
Expand Down
3 changes: 2 additions & 1 deletion moveos/moveos-verifier/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,5 @@ move-vm-runtime = { workspace = true }
move-vm-types = { workspace = true }
move-symbol-pool = { workspace = true }

moveos-types = { workspace = true }
moveos-types = { workspace = true }
log = "0.4.21"
16 changes: 16 additions & 0 deletions moveos/moveos-verifier/src/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,22 @@ pub fn inject_runtime_metadata<P: AsRef<Path>>(
CompiledUnit::Module(named_module) => {
if let Some(module_metadata) = metadata.get(&named_module.module.self_id()) {
if !module_metadata.is_empty() {
log::debug!(
"\n\nstart dump data structs map {:?}",
steelgeek091 marked this conversation as resolved.
Show resolved Hide resolved
named_module.module.self_id().to_string()
);
for (k, v) in module_metadata.data_struct_map.iter() {
log::debug!("{:?} -> {:?}", k, v);
}
log::debug!("\n");
for (k, v) in module_metadata.data_struct_func_map.iter() {
log::debug!("{:?} -> {:?}", k, v);
}
log::debug!(
"start dump data structs map {:?}\n\n",
named_module.module.self_id().to_string()
);

let serialized_metadata =
bcs::to_bytes(&module_metadata).expect("BCS for RuntimeModuleMetadata");
named_module.module.metadata.push(Metadata {
Expand Down
91 changes: 69 additions & 22 deletions moveos/moveos-verifier/src/metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -929,11 +929,16 @@ impl<'a> ExtendedChecker<'a> {

impl<'a> ExtendedChecker<'a> {
fn check_data_struct(&mut self, module_env: &ModuleEnv) {
let mut available_data_structs = BTreeMap::new();

for struct_def in module_env.get_structs() {
if is_data_struct_annotation(&struct_def, module_env) {
if is_copy_drop_struct(&struct_def) {
let (error_message, is_allowed) =
check_data_struct_fields(&struct_def, module_env);
let (error_message, is_allowed) = check_data_struct_fields(
&struct_def,
module_env,
&mut available_data_structs,
);
if !is_allowed {
self.env
.error(&struct_def.get_loc(), error_message.as_str());
Expand All @@ -952,6 +957,20 @@ impl<'a> ExtendedChecker<'a> {
}
}

if !available_data_structs.is_empty() {
log::debug!(
"\n\ncheck_data_struct() module {:?} data structs start...",
module_env.get_full_name_str()
);
for (k, v) in available_data_structs.iter() {
log::debug!("{:?} -> {:?}", k, v);
}
log::debug!(
"\n\ncheck_data_struct() module {:?} data structs end...",
module_env.get_full_name_str()
);
}

let verified_module = match module_env.get_verified_module() {
None => {
self.env
Expand All @@ -963,29 +982,30 @@ impl<'a> ExtendedChecker<'a> {

let module_metadata = self.output.entry(verified_module.self_id()).or_default();

let data_struct_map = unsafe {
let result: BTreeMap<String, bool> = GLOBAL_DATA_STRUCT
.iter()
.map(|(key, value)| (key.clone(), *value))
.collect();
result
};
let data_struct_map: BTreeMap<String, bool> = available_data_structs
.iter()
.map(|(key, value)| (key.clone(), *value))
.collect();

module_metadata.data_struct_map = data_struct_map;

check_data_struct_func(self, module_env);
}
}

fn check_data_struct_fields(struct_def: &StructEnv, module_env: &ModuleEnv) -> (String, bool) {
fn check_data_struct_fields(
struct_def: &StructEnv,
module_env: &ModuleEnv,
valid_structs: &mut BTreeMap<String, bool>,
) -> (String, bool) {
let struct_fields = struct_def.get_fields().collect_vec();
for field in struct_fields {
let field_type = field.get_type();
let field_name = module_env
.symbol_pool()
.string(field.get_name())
.to_string();
let is_allowed = check_data_struct_fields_type(&field_type, module_env);
let is_allowed = check_data_struct_fields_type(&field_type, module_env, valid_structs);
if !is_allowed {
let struct_name = module_env
.symbol_pool()
Expand All @@ -1008,17 +1028,24 @@ fn check_data_struct_fields(struct_def: &StructEnv, module_env: &ModuleEnv) -> (
.string(struct_def.get_name())
.to_string();
let full_struct_name = format!("{}::{}", module_env.get_full_name_str(), struct_name);
valid_structs.insert(full_struct_name.clone(), true);
unsafe {
GLOBAL_DATA_STRUCT.insert(full_struct_name, true);
}

("".to_string(), true)
}

fn check_data_struct_fields_type(field_type: &Type, module_env: &ModuleEnv) -> bool {
fn check_data_struct_fields_type(
field_type: &Type,
module_env: &ModuleEnv,
valid_structs: &mut BTreeMap<String, bool>,
) -> bool {
return match field_type {
Type::Primitive(_) => true,
Type::Vector(item_type) => check_data_struct_fields_type(item_type.as_ref(), module_env),
Type::Vector(item_type) => {
check_data_struct_fields_type(item_type.as_ref(), module_env, valid_structs)
}
Type::Struct(module_id, struct_id, ty_args) => {
let module = module_env.env.get_module(*module_id);

Expand All @@ -1037,7 +1064,7 @@ fn check_data_struct_fields_type(field_type: &Type, module_env: &ModuleEnv) -> b

if is_std_option_type(&full_struct_name) {
if let Some(item_type) = ty_args.first() {
return check_data_struct_fields_type(item_type, module_env);
return check_data_struct_fields_type(item_type, module_env, valid_structs);
}

return false;
Expand All @@ -1051,7 +1078,7 @@ fn check_data_struct_fields_type(field_type: &Type, module_env: &ModuleEnv) -> b
return false;
}

check_data_struct_fields(&struct_env, &struct_module);
check_data_struct_fields(&struct_env, &struct_module, valid_structs);

let is_allowed_opt = unsafe { GLOBAL_DATA_STRUCT.get(full_struct_name.as_str()) };
return if let Some(is_allowed) = is_allowed_opt {
Expand Down Expand Up @@ -1248,13 +1275,24 @@ fn check_data_struct_func(extended_checker: &mut ExtendedChecker, module_env: &M
.entry(compiled_module.self_id())
.or_default();

let data_struct_func_map = unsafe {
let result: BTreeMap<String, Vec<usize>> = GLOBAL_DATA_STRUCT_FUNC
.iter()
.map(|(key, value)| (key.clone(), value.clone()))
.collect();
result
};
let data_struct_func_map: BTreeMap<String, Vec<usize>> = type_name_indices
.iter()
.map(|(key, value)| (key.clone(), value.clone()))
.collect();

if !data_struct_func_map.is_empty() {
log::debug!(
"\n\ncheck_data_struct_func() module {:?} data structs func start...",
module_env.get_full_name_str()
);
for (k, v) in data_struct_func_map.iter() {
log::debug!("{:?} -> {:?}", k, v);
}
log::debug!(
"\n\ncheck_data_struct_func() module {:?} data structs func end...",
module_env.get_full_name_str()
);
}

module_metadata.data_struct_func_map = data_struct_func_map;

Expand Down Expand Up @@ -1295,6 +1333,15 @@ fn check_data_struct_func(extended_checker: &mut ExtendedChecker, module_env: &M
};

if let Some(data_struct_func_indicies) = data_struct_func_types {
let caller_fun_name = fun.get_full_name_str();
log::debug!(
"function {:?}::{:?} call data_struct func {:?} with types {:?}",
module_env.self_address(),
caller_fun_name,
full_path_func_name,
data_struct_func_indicies
);

for generic_type_index in data_struct_func_indicies {
let type_arg = match type_arguments.get(generic_type_index) {
None => {
Expand Down
Loading
Loading