Skip to content

Commit

Permalink
[move][aptos-vm] End-to-end module publishing
Browse files Browse the repository at this point in the history 
- MoveVM now provides an ability to stage modules into a
  temporary storage, verifying is this is "publishable".
- Tests and test harnesses adapted to reduce the number of
  changes (compared to `main`), e.g., old publishing workflows
  are returned but marked as deprecated.
- Tests adapted to work with both V1 and V2 loader publishing
  checks.
- AptosVM now should correctly publish packages in non-block
  context and without custom verification extensions.
  • Loading branch information
@georgemitenkov
georgemitenkov committed Sep 5, 2024
1 parent 6ff9ff2 commit 84a1ae7
Show file tree
Hide file tree
Showing 31 changed files with 1,302 additions and 999 deletions.
3 changes: 0 additions & 3 deletions aptos-move/aptos-vm-types/src/module_and_script_storage/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,3 @@ pub mod script_storage;

mod state_view_adapter;
pub use state_view_adapter::{AptosCodeStorageAdapter, AsAptosCodeStorage};

mod temporary_module_storage;
pub use temporary_module_storage::TemporaryModuleStorage;
8 changes: 8 additions & 0 deletions aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ impl<'s, S: StateView> ModuleStorage for AptosCodeStorageAdapter<'s, S> {
self.storage.check_module_exists(address, module_name)
}

fn fetch_module_bytes(
&self,
address: &AccountAddress,
module_name: &IdentStr,
) -> PartialVMResult<Option<Bytes>> {
self.storage.fetch_module_bytes(address, module_name)
}

fn fetch_module_size_in_bytes(
&self,
address: &AccountAddress,
Expand Down
149 changes: 0 additions & 149 deletions aptos-move/aptos-vm-types/src/module_and_script_storage/temporary_module_storage.rs
View file

This file was deleted.

6 changes: 5 additions & 1 deletion aptos-move/aptos-vm-types/src/module_write_set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,12 @@ impl ModuleWriteSet {
self.has_writes_to_special_address
}

pub fn is_empty(&self) -> bool {
self.write_ops().is_empty()
}

pub fn is_empty_or_invariant_violation(&self) -> PartialVMResult<()> {
if !self.write_ops().is_empty() {
if !self.is_empty() {
return Err(PartialVMError::new(
StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR,
));
Expand Down
120 changes: 54 additions & 66 deletions aptos-move/aptos-vm/src/aptos_vm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ use aptos_vm_types::{
environment::Environment,
module_and_script_storage::{
module_storage::AptosModuleStorage, script_storage::AptosScriptStorage,
AptosCodeStorageAdapter, AsAptosCodeStorage, TemporaryModuleStorage,
AptosCodeStorageAdapter, AsAptosCodeStorage,
},
module_write_set::ModuleWriteSet,
output::VMOutput,
Expand Down Expand Up @@ -111,6 +111,7 @@ use move_core_types::{
use move_vm_runtime::{
logging::expect_no_verification_errors,
module_traversal::{TraversalContext, TraversalStorage},
TemporaryModuleStorage, UnreachableCodeStorage,
};
use move_vm_types::gas::{GasMeter, UnmeteredGasMeter};
use num_cpus;
Expand Down Expand Up @@ -1452,11 +1453,10 @@ impl AptosVM {
Ok(epilogue_session)
}

/// Execute all module initializers.
/// Execute all module initializers. This code is only used for V1 loader implementation.
fn execute_module_initialization(
&self,
session: &mut SessionExt,
module_storage: &impl AptosModuleStorage,
gas_meter: &mut impl AptosGasMeter,
modules: &[CompiledModule],
exists: BTreeSet<ModuleId>,
Expand All @@ -1471,8 +1471,12 @@ impl AptosVM {
continue;
}
*new_published_modules_loaded = true;
let init_function =
session.load_function(module_storage, &module.self_id(), init_func_name, &[]);
let init_function = session.load_function(
&UnreachableCodeStorage,
&module.self_id(),
init_func_name,
&[],
);
// it is ok to not have init_module function
// init_module function should be (1) private and (2) has no return value
// Note that for historic reasons, verification here is treated
Expand All @@ -1491,7 +1495,7 @@ impl AptosVM {
args,
gas_meter,
traversal_context,
module_storage,
&UnreachableCodeStorage,
)?;
} else {
return Err(PartialVMError::new(StatusCode::CONSTRAINT_NOT_SATISFIED)
Expand Down Expand Up @@ -1656,38 +1660,24 @@ impl AptosVM {
let compat = Compatibility::new(check_struct_layout, check_friend_linking);

if self.features().use_loader_v2() {
// TODO(loader_v2): This check MUST also have inside of it whatever
// `validate_publish_request` was doing.
// Also, uncomment when the implementation ready, for now we
// do not need this to block e2e happy test cases.
// session.verify_module_bundle_before_publishing_with_compat_config(
// modules.as_slice(),
// &destination,
// module_storage,
// compat,
// )?;

// Create module write set for modules to be published.
let write_ops = session
.convert_modules_into_write_ops(
module_storage,
bundle.iter().zip(modules).map(|(m, compiled_module)| {
(m.code().to_vec().into(), compiled_module)
}),
)
.map_err(|e| e.finish(Location::Undefined))?;

// Create temporary VM, session, and module storage for running init_module.
// We create a new VM so that type cache is not inconsistent when init_module
// function fails, but loads some new types , type depth formulas, or struct
// name indices.
// TODO(loader_v2): Revisit this to support in a nicer way?
let tmp_module_storage = TemporaryModuleStorage::create(
self.move_vm.runtime_environment(),
write_ops,
module_storage,
);
// Create a temporary storage. If this fails, it means publishing
// is not possible. We use a new VM here so that struct index map
// in the environment, and the type cache inside loader are not
// inconsistent.
// TODO(loader_v2): Revisit this to make it less ugly.
let tmp_vm = self.move_vm.clone();
let tmp_module_storage = TemporaryModuleStorage::new_with_compat_config(
&destination,
// TODO(loader_v2): Make sure to `validate_publish_request` passes to environment
// verifier extension.
tmp_vm.runtime_environment(),
compat,
module_storage,
bundle.into_bytes(),
)
.map_err(|e| e.finish(Location::Undefined))?;

// Run init_module for each new module. We use a new session for that as well.
let mut tmp_session = tmp_vm.new_session(
resolver,
SessionId::txn_meta(transaction_metadata),
Expand All @@ -1705,35 +1695,35 @@ impl AptosVM {
}

let module_id = module.self_id();
let init_function = tmp_session.load_function(
&tmp_module_storage,
&module_id,
init_func_name,
&[],
);
if init_function.is_ok() {
if verifier::module_init::verify_module_init_function(module).is_ok() {
tmp_session.execute_function_bypass_visibility(
&module_id,
init_func_name,
vec![],
vec![MoveValue::Signer(destination)
.simple_serialize()
.unwrap()],
gas_meter,
traversal_context,
&tmp_module_storage,
)?;
} else {
// TODO(loader_v2): Use this opportunity to change the error message?
return Err(PartialVMError::new(
StatusCode::CONSTRAINT_NOT_SATISFIED,
)
.finish(Location::Undefined));
}
let init_function_exists = tmp_session
.load_function(&tmp_module_storage, &module_id, init_func_name, &[])
.is_ok();
if init_function_exists {
verifier::module_init::verify_module_init_function(module)
.map_err(|e| e.finish(Location::Undefined))?;
tmp_session.execute_function_bypass_visibility(
&module_id,
init_func_name,
vec![],
vec![MoveValue::Signer(destination).simple_serialize().unwrap()],
gas_meter,
traversal_context,
&tmp_module_storage,
)?;
}
}

// Create module write set for modules to be published. We do not care
// here about writes to special addresses because there is no flushing.
let write_ops = session
.convert_modules_into_write_ops(
module_storage,
tmp_module_storage.release_verified_module_bundle(),
)
.map_err(|e| e.finish(Location::Undefined))?;
let module_write_set = ModuleWriteSet::new(false, write_ops);

// Process init_module side-effects.
let (init_module_changes, empty_write_set) =
tmp_session.finish(change_set_configs, module_storage)?;
empty_write_set
Expand All @@ -1743,7 +1733,7 @@ impl AptosVM {
.finish(Location::Undefined)
})?;

Ok(Some((tmp_module_storage.destroy(), init_module_changes)))
Ok(Some((module_write_set, init_module_changes)))
} else {
// Validate the module bundle
self.validate_publish_request(
Expand Down Expand Up @@ -1771,13 +1761,11 @@ impl AptosVM {
bundle.into_inner(),
destination,
gas_meter,
module_storage,
compat,
)?;

self.execute_module_initialization(
session,
module_storage,
gas_meter,
modules,
exists,
Expand Down
4 changes: 2 additions & 2 deletions aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,10 @@ impl<'r, 'l> SessionExt<'r, 'l> {
}
}

pub fn convert_modules_into_write_ops<'a>(
pub fn convert_modules_into_write_ops(
&self,
module_storage: &impl AptosModuleStorage,
code_and_modules: impl IntoIterator<Item = (Bytes, &'a CompiledModule)>,
code_and_modules: impl IntoIterator<Item = (Bytes, CompiledModule)>,
) -> PartialVMResult<BTreeMap<StateKey, WriteOp>> {
let woc = WriteOpConverter::new(self.resolver, self.is_storage_slot_metadata_enabled);
woc.convert_modules_into_write_ops(module_storage, code_and_modules)
Expand Down
Loading

0 comments on commit 84a1ae7

Please sign in to comment.