cherry-pick: remove usage of normalized types and improve module comp…

…lexity check (#260) (#13936) * improve metadata check efficiency * mark normalized types as deprecated * improve module complexity check * Update timed_features.rs * Add activation time * Update feature flag name * [release-1.15] Deactivate broken package overrides (#13797) Renaming the tomls for deactivating the tests * Update timed_features.rs --------- Co-authored-by: runtianz <[email protected]> Co-authored-by: Wolfgang Grieskamp <[email protected]>
aptos-labs · Jul 8, 2024 · a9b8526 · a9b8526
1 parent 46a1482
commit a9b8526
Show file tree

Hide file tree

Showing 20 changed files with 564 additions and 36 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -249,6 +249,7 @@ members = [
     "third_party/move/tools/move-resource-viewer",
     "third_party/move/tools/move-unit-test",
     "tools/calc-dep-sizes",
+    "tools/compute-module-expansion-size",
     "types",
     "vm-validator",
 ]

diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs
@@ -50,7 +50,7 @@ use aptos_types::{
     move_utils::as_move_value::AsMoveValue,
     on_chain_config::{
         new_epoch_event_key, ApprovedExecutionHashes, ConfigStorage, FeatureFlag, Features,
-        OnChainConfig, TimedFeatures,
+        OnChainConfig, TimedFeatureFlag, TimedFeatures,
     },
     randomness::Randomness,
     state_store::{StateView, TStateView},
@@ -1431,6 +1431,18 @@ impl AptosVM {
                 // TODO: Revisit the order of traversal. Consider switching to alphabetical order.
             }
 
+            if self
+                .timed_features()
+                .is_enabled(TimedFeatureFlag::ModuleComplexityCheck)
+            {
+                for (module, blob) in modules.iter().zip(bundle.iter()) {
+                    // TODO(Gas): Make budget configurable.
+                    let budget = 2048 + blob.code().len() as u64 * 20;
+                    move_binary_format::check_complexity::check_module_complexity(module, budget)
+                        .map_err(|err| err.finish(Location::Undefined))?;
+                }
+            }
+
             // Validate the module bundle
             self.validate_publish_request(session, modules, expected_modules, allowed_deps)?;
 

diff --git a/aptos-move/aptos-vm/src/lib.rs b/aptos-move/aptos-vm/src/lib.rs
@@ -3,6 +3,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 #![forbid(unsafe_code)]
+#![deny(deprecated)]
 
 //! # The VM runtime
 //!

diff --git a/aptos-move/aptos-vm/src/verifier/resource_groups.rs b/aptos-move/aptos-vm/src/verifier/resource_groups.rs
@@ -6,7 +6,6 @@ use aptos_framework::{ResourceGroupScope, RuntimeModuleMetadataV1};
 use move_binary_format::{
     access::ModuleAccess,
     errors::{Location, PartialVMError, VMError, VMResult},
-    normalized::Struct,
     CompiledModule,
 };
 use move_core_types::{
@@ -172,7 +171,11 @@ pub(crate) fn extract_resource_group_metadata_from_module(
         let structs = module
             .struct_defs()
             .iter()
-            .map(|d| Struct::new(&module, d).0.into_string())
+            .map(|struct_def| {
+                let struct_handle = module.struct_handle_at(struct_def.struct_handle);
+                let name = module.identifier_at(struct_handle.name).to_string();
+                name
+            })
             .collect::<BTreeSet<_>>();
         Ok((groups, members, structs))
     } else {

diff --git a/aptos-move/framework/src/module_metadata.rs b/aptos-move/framework/src/module_metadata.rs
@@ -3,22 +3,21 @@
 
 use crate::extended_checks::ResourceGroupScope;
 use aptos_types::{
-    on_chain_config::{FeatureFlag, Features, TimedFeatures},
+    on_chain_config::{FeatureFlag, Features, TimedFeatureFlag, TimedFeatures},
     transaction::AbortInfo,
 };
 use lru::LruCache;
 use move_binary_format::{
     access::ModuleAccess,
     file_format::{
-        Ability, AbilitySet, CompiledScript, IdentifierIndex, SignatureToken,
-        StructFieldInformation, TableIndex,
+        Ability, AbilitySet, CompiledScript, FunctionDefinition, FunctionHandle, IdentifierIndex,
+        SignatureToken, StructDefinition, StructFieldInformation, StructHandle, TableIndex,
     },
-    normalized::{Function, Struct},
     CompiledModule,
 };
 use move_core_types::{
     errmap::ErrorDescription,
-    identifier::Identifier,
+    identifier::{IdentStr, Identifier},
     language_storage::{ModuleId, StructTag},
     metadata::Metadata,
 };
@@ -388,12 +387,12 @@ pub struct AttributeValidationError {
 }
 
 pub fn is_valid_unbiasable_function(
-    functions: &BTreeMap<Identifier, Function>,
+    functions: &BTreeMap<&IdentStr, (&FunctionHandle, &FunctionDefinition)>,
     fun: &str,
 ) -> Result<(), AttributeValidationError> {
     if let Ok(ident_fun) = Identifier::new(fun) {
-        if let Some(f) = functions.get(&ident_fun) {
-            if f.is_entry && !f.visibility.is_public() {
+        if let Some((_func_handle, func_def)) = functions.get(ident_fun.as_ident_str()) {
+            if func_def.is_entry && !func_def.visibility.is_public() {
                 return Ok(());
             }
         }
@@ -406,12 +405,14 @@ pub fn is_valid_unbiasable_function(
 }
 
 pub fn is_valid_view_function(
-    functions: &BTreeMap<Identifier, Function>,
+    module: &CompiledModule,
+    functions: &BTreeMap<&IdentStr, (&FunctionHandle, &FunctionDefinition)>,
     fun: &str,
 ) -> Result<(), AttributeValidationError> {
     if let Ok(ident_fun) = Identifier::new(fun) {
-        if let Some(mod_fun) = functions.get(&ident_fun) {
-            if !mod_fun.return_.is_empty() {
+        if let Some((func_handle, _func_def)) = functions.get(ident_fun.as_ident_str()) {
+            let sig = module.signature_at(func_handle.return_);
+            if !sig.0.is_empty() {
                 return Ok(());
             }
         }
@@ -424,14 +425,18 @@ pub fn is_valid_view_function(
 }
 
 pub fn is_valid_resource_group(
-    structs: &BTreeMap<Identifier, Struct>,
+    structs: &BTreeMap<&IdentStr, (&StructHandle, &StructDefinition)>,
     struct_: &str,
 ) -> Result<(), AttributeValidationError> {
     if let Ok(ident_struct) = Identifier::new(struct_) {
-        if let Some(mod_struct) = structs.get(&ident_struct) {
-            if mod_struct.abilities == AbilitySet::EMPTY
-                && mod_struct.type_parameters.is_empty()
-                && mod_struct.fields.len() == 1
+        if let Some((struct_handle, struct_def)) = structs.get(ident_struct.as_ident_str()) {
+            let num_fields = match &struct_def.field_information {
+                StructFieldInformation::Native => 0,
+                StructFieldInformation::Declared(fields) => fields.len(),
+            };
+            if struct_handle.abilities == AbilitySet::EMPTY
+                && struct_handle.type_parameters.is_empty()
+                && num_fields == 1
             {
                 return Ok(());
             }
@@ -445,12 +450,12 @@ pub fn is_valid_resource_group(
 }
 
 pub fn is_valid_resource_group_member(
-    structs: &BTreeMap<Identifier, Struct>,
+    structs: &BTreeMap<&IdentStr, (&StructHandle, &StructDefinition)>,
     struct_: &str,
 ) -> Result<(), AttributeValidationError> {
     if let Ok(ident_struct) = Identifier::new(struct_) {
-        if let Some(mod_struct) = structs.get(&ident_struct) {
-            if mod_struct.abilities.has_ability(Ability::Key) {
+        if let Some((struct_handle, _struct_def)) = structs.get(ident_struct.as_ident_str()) {
+            if struct_handle.abilities.has_ability(Ability::Key) {
                 return Ok(());
             }
         }
@@ -465,9 +470,11 @@ pub fn is_valid_resource_group_member(
 pub fn verify_module_metadata(
     module: &CompiledModule,
     features: &Features,
-    _timed_features: &TimedFeatures,
+    timed_features: &TimedFeatures,
 ) -> Result<(), MetaDataValidationError> {
-    if features.is_enabled(FeatureFlag::SAFER_METADATA) {
+    if features.is_enabled(FeatureFlag::SAFER_METADATA)
+        && timed_features.is_enabled(TimedFeatureFlag::ModuleComplexityCheck)
+    {
         check_module_complexity(module)?;
     }
 
@@ -483,13 +490,17 @@ pub fn verify_module_metadata(
     let functions = module
         .function_defs
         .iter()
-        .map(|func_def| Function::new(module, func_def))
+        .map(|func_def| {
+            let func_handle = module.function_handle_at(func_def.function);
+            let name = module.identifier_at(func_handle.name);
+            (name, (func_handle, func_def))
+        })
         .collect::<BTreeMap<_, _>>();
 
     for (fun, attrs) in &metadata.fun_attributes {
         for attr in attrs {
             if attr.is_view_function() {
-                is_valid_view_function(&functions, fun)?;
+                is_valid_view_function(module, &functions, fun)?;
             } else if attr.is_randomness() {
                 is_valid_unbiasable_function(&functions, fun)?;
             } else {
@@ -505,7 +516,11 @@ pub fn verify_module_metadata(
     let structs = module
         .struct_defs
         .iter()
-        .map(|d| Struct::new(module, d))
+        .map(|struct_def| {
+            let struct_handle = module.struct_handle_at(struct_def.struct_handle);
+            let name = module.identifier_at(struct_handle.name);
+            (name, (struct_handle, struct_def))
+        })
         .collect::<BTreeMap<_, _>>();
 
     for (struct_, attrs) in &metadata.struct_attributes {