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

…lexity check (aptos-labs#260)

* 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 (aptos-labs#13797)

Renaming the tomls for deactivating the tests

* Update timed_features.rs

---------

Co-authored-by: runtianz <runtian@aptoslabs.com>
Co-authored-by: Wolfgang Grieskamp <wg@aptoslabs.com>

Cargo.toml

@@ -248,6 +248,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",
 ]

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},
@@ -1420,6 +1420,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)?;
 

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
 //!

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 {

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 {