Skip to content

Commit

Permalink
[test] Remove conditionals due to enabled features
Browse files Browse the repository at this point in the history
  • Loading branch information
georgemitenkov committed Jan 22, 2025
1 parent 95190dc commit b7e7d19
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 62 deletions.
6 changes: 0 additions & 6 deletions aptos-move/aptos-vm-environment/src/prod_configs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,6 @@ pub fn aptos_prod_vm_config(
let deserializer_config = aptos_prod_deserializer_config(features);
let verifier_config = aptos_prod_verifier_config(features);

// Compatibility checker v2 is enabled either by its own flag or if enum types are enabled.
let use_compatibility_checker_v2 = verifier_config.enable_enum_types
|| features.is_enabled(FeatureFlag::USE_COMPATIBILITY_CHECKER_V2);

VMConfig {
verifier_config,
deserializer_config,
Expand All @@ -142,8 +138,6 @@ pub fn aptos_prod_vm_config(
// manually where applicable.
delayed_field_optimization_enabled: false,
ty_builder,
disallow_dispatch_for_native: features.is_enabled(FeatureFlag::DISALLOW_USER_NATIVES),
use_compatibility_checker_v2,
use_loader_v2: features.is_loader_v2_enabled(),
use_call_tree_and_instruction_cache: features
.is_call_tree_and_instruction_vm_cache_enabled(),
Expand Down
13 changes: 2 additions & 11 deletions aptos-move/aptos-vm/src/aptos_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -893,11 +893,7 @@ impl AptosVM {
)?;

// Native entry function is forbidden.
if self
.features()
.is_enabled(FeatureFlag::DISALLOW_USER_NATIVES)
&& function.is_native()
{
if function.is_native() {
return Err(
PartialVMError::new(StatusCode::USER_DEFINED_NATIVE_NOT_ALLOWED)
.with_message(
Expand Down Expand Up @@ -1775,12 +1771,7 @@ impl AptosVM {
self.reject_unstable_bytecode(modules)?;
}

if self
.features()
.is_enabled(FeatureFlag::DISALLOW_USER_NATIVES)
{
verifier::native_validation::validate_module_natives(modules)?;
}
verifier::native_validation::validate_module_natives(modules)?;

for m in modules {
if !expected_modules.remove(m.self_id().name().as_str()) {
Expand Down
5 changes: 1 addition & 4 deletions aptos-move/framework/src/natives/function_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,7 @@ fn native_check_dispatch_type_compatibility_impl(
&& rhs.return_tys() == lhs.return_tys()
&& &lhs.param_tys()[0..lhs.param_count() - 1] == rhs.param_tys()
&& !rhs.is_friend_or_private()
&& (!context
.get_feature_flags()
.is_enabled(aptos_types::on_chain_config::FeatureFlag::DISALLOW_USER_NATIVES)
|| !rhs.is_native())
&& !rhs.is_native()
&& lhs_id != rhs_id
)])
}
Expand Down
4 changes: 0 additions & 4 deletions third_party/move/move-vm/runtime/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ pub struct VMConfig {
pub type_byte_cost: u64,
pub delayed_field_optimization_enabled: bool,
pub ty_builder: TypeBuilder,
pub disallow_dispatch_for_native: bool,
pub use_compatibility_checker_v2: bool,
pub use_loader_v2: bool,
pub use_call_tree_and_instruction_cache: bool,
}
Expand All @@ -43,8 +41,6 @@ impl Default for VMConfig {
type_byte_cost: 0,
delayed_field_optimization_enabled: false,
ty_builder: TypeBuilder::with_limits(128, 20),
disallow_dispatch_for_native: true,
use_compatibility_checker_v2: true,
use_loader_v2: true,
use_call_tree_and_instruction_cache: true,
}
Expand Down
2 changes: 1 addition & 1 deletion third_party/move/move-vm/runtime/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@ impl InterpreterImpl {
));
}

if resolver.vm_config().disallow_dispatch_for_native && target_func.is_native() {
if target_func.is_native() {
return Err(PartialVMError::new(StatusCode::RUNTIME_DISPATCH_ERROR)
.with_message("Invoking native function during dispatch".to_string()));
}
Expand Down
23 changes: 5 additions & 18 deletions third_party/move/move-vm/runtime/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use move_binary_format::{
compatibility::Compatibility,
errors::{verification_error, Location, PartialVMError, PartialVMResult, VMResult},
file_format::LocalIndex,
normalized, CompiledModule, IndexKind,
CompiledModule, IndexKind,
};
use move_core_types::{
account_address::AccountAddress, language_storage::TypeTag, value::MoveTypeLayout,
Expand Down Expand Up @@ -143,23 +143,10 @@ impl VMRuntime {

#[allow(deprecated)]
if data_store.exists_module(&module_id)? && compat.need_check_compat() {
let old_module_ref = loader.load_module(&module_id, data_store, module_store)?;
let old_module = old_module_ref.as_ref().as_ref();
if loader.vm_config().use_compatibility_checker_v2 {
compat
.check(old_module_ref.as_ref().as_ref(), module)
.map_err(|e| e.finish(Location::Undefined))?
} else {
#[allow(deprecated)]
let old_m = normalized::Module::new(old_module)
.map_err(|e| e.finish(Location::Undefined))?;
#[allow(deprecated)]
let new_m = normalized::Module::new(module)
.map_err(|e| e.finish(Location::Undefined))?;
compat
.legacy_check(&old_m, &new_m)
.map_err(|e| e.finish(Location::Undefined))?
}
let old_module = loader.load_module(&module_id, data_store, module_store)?;
compat
.check(&old_module, module)
.map_err(|e| e.finish(Location::Undefined))?
}
if !bundle_unverified.insert(module_id) {
return Err(PartialVMError::new(StatusCode::DUPLICATE_MODULE_NAME)
Expand Down
23 changes: 5 additions & 18 deletions third_party/move/move-vm/runtime/src/storage/publishing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use move_binary_format::{
access::ModuleAccess,
compatibility::Compatibility,
errors::{verification_error, Location, PartialVMError, PartialVMResult, VMResult},
normalized, CompiledModule, IndexKind,
CompiledModule, IndexKind,
};
use move_core_types::{
account_address::AccountAddress,
Expand Down Expand Up @@ -154,25 +154,12 @@ impl<'a, M: ModuleStorage> StagingModuleStorage<'a, M> {
// All modules can be republished, as long as the new module is compatible
// with the old module.
if compatibility.need_check_compat() {
if let Some(old_module_ref) =
if let Some(old_module) =
existing_module_storage.fetch_deserialized_module(addr, name)?
{
let old_module = old_module_ref.as_ref();
if runtime_environment.vm_config().use_compatibility_checker_v2 {
compatibility
.check(old_module, &compiled_module)
.map_err(|e| e.finish(Location::Undefined))?;
} else {
#[allow(deprecated)]
let old_m = normalized::Module::new(old_module)
.map_err(|e| e.finish(Location::Undefined))?;
#[allow(deprecated)]
let new_m = normalized::Module::new(&compiled_module)
.map_err(|e| e.finish(Location::Undefined))?;
compatibility
.legacy_check(&old_m, &new_m)
.map_err(|e| e.finish(Location::Undefined))?;
}
compatibility
.check(&old_module, &compiled_module)
.map_err(|e| e.finish(Location::Undefined))?;
}
}

Expand Down

0 comments on commit b7e7d19

Please sign in to comment.