Skip to content

Commit

Permalink
[feature] Remove disable publishing in init module
Browse files Browse the repository at this point in the history
  • Loading branch information
georgemitenkov committed Feb 19, 2025
1 parent 3d7c937 commit ae84e30
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ impl From<FeatureFlag> for AptosFeatureFlag {
FeatureFlag::NativeMemoryOperations => AptosFeatureFlag::NATIVE_MEMORY_OPERATIONS,
FeatureFlag::EnableLoaderV2 => AptosFeatureFlag::ENABLE_LOADER_V2,
FeatureFlag::DisallowInitModuleToPublishModules => {
AptosFeatureFlag::DISALLOW_INIT_MODULE_TO_PUBLISH_MODULES
AptosFeatureFlag::_DISALLOW_INIT_MODULE_TO_PUBLISH_MODULES
},
FeatureFlag::EnableCallTreeAndInstructionVMCache => {
AptosFeatureFlag::ENABLE_CALL_TREE_AND_INSTRUCTION_VM_CACHE
Expand Down Expand Up @@ -517,7 +517,7 @@ impl From<AptosFeatureFlag> for FeatureFlag {
AptosFeatureFlag::COLLECTION_OWNER => FeatureFlag::CollectionOwner,
AptosFeatureFlag::NATIVE_MEMORY_OPERATIONS => FeatureFlag::NativeMemoryOperations,
AptosFeatureFlag::ENABLE_LOADER_V2 => FeatureFlag::EnableLoaderV2,
AptosFeatureFlag::DISALLOW_INIT_MODULE_TO_PUBLISH_MODULES => {
AptosFeatureFlag::_DISALLOW_INIT_MODULE_TO_PUBLISH_MODULES => {
FeatureFlag::DisallowInitModuleToPublishModules
},
AptosFeatureFlag::ENABLE_CALL_TREE_AND_INSTRUCTION_VM_CACHE => {
Expand Down
7 changes: 1 addition & 6 deletions aptos-move/aptos-vm/src/aptos_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1576,12 +1576,7 @@ impl AptosVM {
new_published_modules_loaded: &mut bool,
change_set_configs: &ChangeSetConfigs,
) -> Result<UserSessionChangeSet, VMStatus> {
let maybe_publish_request = session.execute(|session| {
let disable_publishing = self
.features()
.is_disallow_init_module_to_publish_modules_enabled();
session.extract_publish_request(disable_publishing)
});
let maybe_publish_request = session.execute(|session| session.extract_publish_request());
if maybe_publish_request.is_none() {
let user_change_set = session.finish(change_set_configs, module_storage)?;

Expand Down
4 changes: 2 additions & 2 deletions aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,9 @@ impl<'r, 'l> SessionExt<'r, 'l> {

/// Returns the publish request if it exists. If the provided flag is set to true, disables any
/// subsequent module publish requests.
pub fn extract_publish_request(&mut self, disable: bool) -> Option<PublishRequest> {
pub fn extract_publish_request(&mut self) -> Option<PublishRequest> {
let ctx = self.get_native_extensions().get_mut::<NativeCodeContext>();
ctx.extract_publish_request(disable)
ctx.extract_publish_request()
}

fn populate_v0_resource_group_change_set(
Expand Down
39 changes: 3 additions & 36 deletions aptos-move/e2e-move-tests/src/tests/code_publishing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use aptos_package_builder::PackageBuilder;
use aptos_types::{
account_address::{create_resource_address, AccountAddress},
move_utils::MemberId,
on_chain_config::{FeatureFlag, OnChainConfig},
on_chain_config::FeatureFlag,
transaction::{ExecutionStatus, TransactionPayload, TransactionStatus},
};
use claims::assert_ok;
Expand All @@ -22,7 +22,7 @@ use move_core_types::{
};
use rstest::rstest;
use serde::{Deserialize, Serialize};
use std::{collections::BTreeSet, str::FromStr};
use std::str::FromStr;

// Note: this module uses parameterized tests via the
// [`rstest` crate](https://crates.io/crates/rstest)
Expand Down Expand Up @@ -595,10 +595,7 @@ fn assert_move_abort(status: TransactionStatus, expected_abort_code: u64) {

#[test]
fn test_init_module_should_not_publish_modules() {
let mut h = MoveHarness::new_with_features(
vec![FeatureFlag::DISALLOW_INIT_MODULE_TO_PUBLISH_MODULES],
vec![],
);
let mut h = MoveHarness::new();
let addr = AccountAddress::from_hex_literal("0xcafe").unwrap();
let account = h.new_account_at(addr);

Expand All @@ -611,33 +608,3 @@ fn test_init_module_should_not_publish_modules() {
// The abort code corresponds to EALREADY_REQUESTED.
assert_move_abort(output.status().clone(), 0x03_0000);
}

#[test]
fn test_init_module_can_publish_modules() {
let mut h = MoveHarness::new_with_features(vec![], vec![
FeatureFlag::DISALLOW_INIT_MODULE_TO_PUBLISH_MODULES,
]);
let addr = AccountAddress::from_hex_literal("0xcafe").unwrap();
let account = h.new_account_at(addr);

let txn = h.create_publish_package_cache_building(
&account,
&common::test_dir_path("code_publishing.data/pack_init_module_code_publish"),
|_| {},
);
let output = h.run_block_get_output(vec![txn]).pop().unwrap();
assert_success!(output.status().clone());

let registry: PackageRegistry = h
.read_resource(&addr, PackageRegistry::struct_tag())
.unwrap();
assert_eq!(registry.packages.len(), 2);
let mut package_names = BTreeSet::new();
for package in registry.packages {
package_names.insert(package.name);
}
// The package metadata containing init_module exists.
assert!(package_names.remove("test_package"));
// The package metadata published by init_module call also exists.
assert!(package_names.remove("Package"));
}
6 changes: 2 additions & 4 deletions aptos-move/framework/src/natives/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,12 @@ impl NativeCodeContext {
}
}

pub fn extract_publish_request(&mut self, disable: bool) -> Option<PublishRequest> {
pub fn extract_publish_request(&mut self) -> Option<PublishRequest> {
if !self.enabled {
return None;
}

if disable {
self.enabled = false;
}
self.enabled = false;
self.requested_module_bundle.take()
}
}
Expand Down
10 changes: 3 additions & 7 deletions types/src/on_chain_config/aptos_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,9 @@ pub enum FeatureFlag {
ENABLE_LOADER_V2 = 81,
/// Prior to this feature flag, it was possible to attempt 'init_module' to publish modules
/// that results in a new package created but without any code. With this feature, it is no
/// longer possible and an explicit error is returned if publishing is attempted.
DISALLOW_INIT_MODULE_TO_PUBLISH_MODULES = 82,
/// longer possible and an explicit error is returned if publishing is attempted. The feature
/// was enabled on mainnet and will not be disabled.
_DISALLOW_INIT_MODULE_TO_PUBLISH_MODULES = 82,
/// We keep the Call Tree cache and instruction (per-instruction)
/// cache together here. Generally, we could allow Call Tree
/// cache and disallow instruction cache, however there's little
Expand Down Expand Up @@ -202,7 +203,6 @@ impl FeatureFlag {
FeatureFlag::NATIVE_MEMORY_OPERATIONS,
FeatureFlag::COLLECTION_OWNER,
FeatureFlag::ENABLE_LOADER_V2,
FeatureFlag::DISALLOW_INIT_MODULE_TO_PUBLISH_MODULES,
FeatureFlag::PERMISSIONED_SIGNER,
FeatureFlag::ENABLE_CALL_TREE_AND_INSTRUCTION_VM_CACHE,
FeatureFlag::ACCOUNT_ABSTRACTION,
Expand Down Expand Up @@ -359,10 +359,6 @@ impl Features {
self.is_enabled(FeatureFlag::ENABLE_LOADER_V2)
}

pub fn is_disallow_init_module_to_publish_modules_enabled(&self) -> bool {
self.is_enabled(FeatureFlag::DISALLOW_INIT_MODULE_TO_PUBLISH_MODULES)
}

pub fn is_call_tree_and_instruction_vm_cache_enabled(&self) -> bool {
self.is_enabled(FeatureFlag::ENABLE_CALL_TREE_AND_INSTRUCTION_VM_CACHE)
}
Expand Down

0 comments on commit ae84e30

Please sign in to comment.