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 committed Aug 24, 2024
1 parent c7c3780 commit b484fa3
Show file tree
Hide file tree
Showing 31 changed files with 1,298 additions and 995 deletions.
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;
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)
}

Check warning on line 65 in aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm-types/src/module_and_script_storage/state_view_adapter.rs#L59-L65

Added lines #L59 - L65 were not covered by tests

fn fetch_module_size_in_bytes(
&self,
address: &AccountAddress,
Expand Down

This file was deleted.

6 changes: 5 additions & 1 deletion aptos-move/aptos-vm-types/src/module_write_set.rs
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()
}

Check warning on line 77 in aptos-move/aptos-vm-types/src/module_write_set.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm-types/src/module_write_set.rs#L75-L77

Added lines #L75 - L77 were not covered by tests

pub fn is_empty_or_invariant_violation(&self) -> PartialVMResult<()> {
if !self.write_ops().is_empty() {
if !self.is_empty() {

Check warning on line 80 in aptos-move/aptos-vm-types/src/module_write_set.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm-types/src/module_write_set.rs#L80

Added line #L80 was not covered by tests
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
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,
&[],
);

Check warning on line 1479 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm.rs#L1474-L1479

Added lines #L1474 - L1479 were not covered by tests
// 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,

Check warning on line 1498 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm.rs#L1498

Added line #L1498 was not covered by tests
)?;
} 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))?;

Check warning on line 1678 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm.rs#L1669-L1678

Added lines #L1669 - L1678 were not covered by tests

// 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,
)?;

Check warning on line 1712 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm.rs#L1698-L1712

Added lines #L1698 - L1712 were not covered by tests
}
}

// 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);

Check warning on line 1724 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm.rs#L1718-L1724

Added lines #L1718 - L1724 were not covered by tests

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

Check warning on line 1736 in aptos-move/aptos-vm/src/aptos_vm.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/aptos_vm.rs#L1736

Added line #L1736 was not covered by tests
} 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
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(

Check warning on line 124 in aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs#L124

Added line #L124 was not covered by tests
&self,
module_storage: &impl AptosModuleStorage,
code_and_modules: impl IntoIterator<Item = (Bytes, &'a CompiledModule)>,
code_and_modules: impl IntoIterator<Item = (Bytes, CompiledModule)>,

Check warning on line 127 in aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs

View check run for this annotation

Codecov / codecov/patch

aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs#L127

Added line #L127 was not covered by tests
) -> 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 b484fa3

Please sign in to comment.