Skip to content

Commit

Permalink
[move-vm] Refactor Loader (aptos-labs#9320)
Browse files Browse the repository at this point in the history
* Rename variant

* [move-vm] Store ability in runtime type

* fixup! [move-vm] Store ability in runtime type

* fixup! fixup! [move-vm] Store ability in runtime type

* fixup! fixup! fixup! [move-vm] Store ability in runtime type

* [move_vm] Split loader into multiple files

* [move_vm] Cache struct type for modules

* [move_vm] Move depth cache to type cache

* [move-vm] Use name to replace type index

* [move-vm] Remove function index

* [move-vm] Inline functions

* [move-vm] Remove cached index from definition

* [move-vm] Check cross module linking before loading

* [move-vm] Remove global struct cache

* [move-vm] Inline struct name to avoid excessive memory allocation

* [move-vm] Split function in loader

* [move-vm] Split out module cache

* [e2e-test] Add randomized test for loader

* Fix Lint

* [move-vm] Cache signature resolution

* [move-vm] Removed unneeded signature token

* [move-vm] Arc-ed type argument

* Fix Zekun's comments

* fixup! [e2e-test] Add randomized test for loader

* add comments for the test strategy

* Rename struct name

* More renaming

* Addressing more comments
  • Loading branch information
runtian-zhou authored and Zekun Wang committed Oct 13, 2023
1 parent f6cd004 commit ed3b35e
Show file tree
Hide file tree
Showing 21 changed files with 1,554 additions and 1,356 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 9 additions & 24 deletions aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,7 @@ pub(crate) fn validate_combine_signer_and_txn_args(
let allowed_structs = get_allowed_structs(are_struct_constructors_enabled);
// Need to keep this here to ensure we return the historic correct error code for replay
for ty in func.parameters[signer_param_cnt..].iter() {
let valid = is_valid_txn_arg(
session,
&ty.subst(&func.type_arguments).unwrap(),
allowed_structs,
);
let valid = is_valid_txn_arg(&ty.subst(&func.type_arguments).unwrap(), allowed_structs);
if !valid {
return Err(VMStatus::error(
StatusCode::INVALID_MAIN_FUNCTION_SIGNATURE,
Expand Down Expand Up @@ -186,23 +182,15 @@ pub(crate) fn validate_combine_signer_and_txn_args(
}

// Return whether the argument is valid/allowed and whether it needs construction.
pub(crate) fn is_valid_txn_arg(
session: &SessionExt,
typ: &Type,
allowed_structs: &ConstructorMap,
) -> bool {
pub(crate) fn is_valid_txn_arg(typ: &Type, allowed_structs: &ConstructorMap) -> bool {
use move_vm_types::loaded_data::runtime_types::Type::*;

match typ {
Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address => true,
Vector(inner) => is_valid_txn_arg(session, inner, allowed_structs),
Struct(idx) | StructInstantiation(idx, _) => {
if let Some(st) = session.get_struct_type(*idx) {
let full_name = format!("{}::{}", st.module.short_str_lossless(), st.name);
allowed_structs.contains_key(&full_name)
} else {
false
}
Vector(inner) => is_valid_txn_arg(inner, allowed_structs),
Struct { name, .. } | StructInstantiation { name, .. } => {
let full_name = format!("{}::{}", name.module.short_str_lossless(), name.name);
allowed_structs.contains_key(&full_name)
},
Signer | Reference(_) | MutableReference(_) | TyParam(_) => false,
}
Expand Down Expand Up @@ -254,7 +242,7 @@ fn construct_arg(
use move_vm_types::loaded_data::runtime_types::Type::*;
match ty {
Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address => Ok(arg),
Vector(_) | Struct(_) | StructInstantiation(_, _) => {
Vector(_) | Struct { .. } | StructInstantiation { .. } => {
let mut cursor = Cursor::new(&arg[..]);
let mut new_arg = vec![];
let mut max_invocations = 10; // Read from config in the future
Expand Down Expand Up @@ -322,13 +310,10 @@ pub(crate) fn recursively_construct_arg(
len -= 1;
}
},
Struct(idx) | StructInstantiation(idx, _) => {
Struct { name, .. } | StructInstantiation { name, .. } => {
// validate the struct value, we use `expect()` because that check was already
// performed in `is_valid_txn_arg`
let st = session
.get_struct_type(*idx)
.ok_or_else(invalid_signature)?;
let full_name = format!("{}::{}", st.module.short_str_lossless(), st.name);
let full_name = format!("{}::{}", name.module.short_str_lossless(), name.name);
let constructor = allowed_structs
.get(&full_name)
.ok_or_else(invalid_signature)?;
Expand Down
2 changes: 2 additions & 0 deletions aptos-move/e2e-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@ move-binary-format = { workspace = true }
move-command-line-common = { workspace = true }
move-core-types = { workspace = true }
move-ir-compiler = { workspace = true }
move-model = { workspace = true }
move-vm-types = { workspace = true }
num_cpus = { workspace = true }
once_cell = { workspace = true }
petgraph = "0.5.1"
proptest = { workspace = true }
proptest-derive = { workspace = true }
rand = { workspace = true }
Expand Down
18 changes: 14 additions & 4 deletions aptos-move/e2e-tests/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -855,9 +855,9 @@ impl FakeExecutor {
.to_vec()
}

pub fn exec(
pub fn exec_module(
&mut self,
module_name: &str,
module_id: &ModuleId,
function_name: &str,
type_params: Vec<TypeTag>,
args: Vec<Vec<u8>>,
Expand All @@ -883,7 +883,7 @@ impl FakeExecutor {
let mut session = vm.new_session(&resolver, SessionId::void());
session
.execute_function_bypass_visibility(
&Self::module(module_name),
module_id,
&Self::name(function_name),
type_params,
args,
Expand All @@ -892,7 +892,7 @@ impl FakeExecutor {
.unwrap_or_else(|e| {
panic!(
"Error calling {}.{}: {}",
module_name,
module_id,
function_name,
e.into_vm_status()
)
Expand All @@ -912,6 +912,16 @@ impl FakeExecutor {
self.event_store.extend(events);
}

pub fn exec(
&mut self,
module_name: &str,
function_name: &str,
type_params: Vec<TypeTag>,
args: Vec<Vec<u8>>,
) {
self.exec_module(&Self::module(module_name), function_name, type_params, args)
}

pub fn try_exec(
&mut self,
module_name: &str,
Expand Down
1 change: 1 addition & 0 deletions aptos-move/e2e-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub mod execution_strategies;
pub mod executor;
pub mod gas_costs;
mod golden_outputs;
pub mod loader;
pub mod on_chain_configs;
mod proptest_types;

Expand Down
186 changes: 186 additions & 0 deletions aptos-move/e2e-tests/src/loader/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
// Copyright © Aptos Foundation
// Parts of the project are originally copyright © Meta Platforms, Inc.
// SPDX-License-Identifier: Apache-2.0

//! Logic for account universes. This is not in the parent module to enforce privacy.

use crate::{account::AccountData, executor::FakeExecutor};
use aptos_proptest_helpers::Index;
use move_core_types::{identifier::Identifier, language_storage::ModuleId};
use move_ir_compiler::Compiler;
use petgraph::{algo::toposort, Direction, Graph};
use proptest::{
collection::{vec, SizeRange},
prelude::*,
};
use std::{cmp::Ordering, collections::HashMap};

mod module_generator;

#[derive(Debug)]
pub struct Node {
name: ModuleId,
self_value: u64,
account_data: AccountData,
}

#[derive(Debug)]
pub struct DependencyGraph(Graph<Node, ()>);

// This module generates a sets of modules that could be used to test the loader.
//
// To generate the module, a DAG is first generated. The node in this graph represents a module and the edge in this graph represents a dependency
// between modules. For example if you generate the following DAG:
//
// M1
// / \
// M2 M3
//
// This will generate three modules: M1, M2 and M3. Where each module will look like following:
//
// module 0x3cf3846156a51da7251ccc84b5e4be10c5ab33049b7692d1164fd2c73ef3638b.M2 {
// public entry foo(): u64 {
// let a: u64;
// label b0:
// a = 49252; // randomly generated integer for self node.
// assert(copy(a) == 49252, 42); // assert execution result matches expected value. Expected value can be derived from traversing the DAG.
// return move(a);
// }
// }
//
// module e57d457d2ffacab8f46dea9779f8ad8135c68fd3574eb2902b3da0cfa67cf9d1.M3 {
// public entry foo(): u64 {
// let a: u64;
// label b0:
// a = 41973; // randomly generated integer for self node.
// assert(copy(a) == 41973, 42); // assert execution result matches expected value. Expected value can be derived from traversing the DAG.
// return move(a);
// }
// }
//
// M2 and M3 are leaf nodes so the it should be a no-op function call. M1 will look like following:
//
// module 0x61da74259dae2c25beb41d11e43c6f5c00cc72d151d8a4f382b02e4e6c420d17.M1 {
// import 0x3cf3846156a51da7251ccc84b5e4be10c5ab33049b7692d1164fd2c73ef3638b.M2;
// import e57d457d2ffacab8f46dea9779f8ad8135c68fd3574eb2902b3da0cfa67cf9d1.M3;
// public entry foo(): u64 {
// let a: u64;
// label b0:
// a = 27856 + M2.foo()+ M3.foo(); // The dependency edge will be converted invocation into dependent module.
// assert(copy(a) == 119081, 42); // The expected value is the sum of self and all its dependent modules.
// return move(a);
// }
// }
//
// By using this strategy, we can generate a set of modules with complex depenency relationship and assert that the new loader is always
// linking the call to the right module.
//
// TODOs:
// - Use Move source language rather than MVIR to generate the modules.
// - randomly generate module upgrade request to change the structure of DAG to make sure the VM will be able to handle
// invaldation properly.
impl DependencyGraph {
/// Returns a [`Strategy`] that generates a universe of accounts with pre-populated initial
/// balances.
///
/// Note that the real number of edges might be smaller than the size input provided due to our way of generating DAG.
/// For example, if a (A, B) and (B, A) are both generated by the strategy, only one of them will be added to the graph.
pub fn strategy(
num_accounts: impl Into<SizeRange>,
expected_num_edges: impl Into<SizeRange>,
balance_strategy: impl Strategy<Value = u64>,
) -> impl Strategy<Value = Self> {
(
vec(
(
AccountData::strategy(balance_strategy),
any::<u16>(),
any::<Identifier>(),
),
num_accounts,
),
vec(any::<(Index, Index)>(), expected_num_edges),
)
.prop_map(move |(accounts, edge_indices)| Self::create(accounts, edge_indices))
}

fn create(accounts: Vec<(AccountData, u16, Identifier)>, edges: Vec<(Index, Index)>) -> Self {
let mut graph = Graph::new();
let indices = accounts
.into_iter()
.map(|(account_data, self_value, module_name)| {
graph.add_node(Node {
name: ModuleId::new(*account_data.address(), module_name),
self_value: self_value as u64,
account_data,
})
})
.collect::<Vec<_>>();
for (lhs_idx, rhs_idx) in edges {
let lhs = lhs_idx.get(&indices);
let rhs = rhs_idx.get(&indices);

match lhs.cmp(rhs) {
Ordering::Greater => {
graph.add_edge(lhs.to_owned(), rhs.to_owned(), ());
},
Ordering::Less => {
graph.add_edge(rhs.to_owned(), lhs.to_owned(), ());
},
Ordering::Equal => (),
}
}
Self(graph)
}

/// Set up [`DepGraph`] with the initial state generated in this universe.
pub fn setup(&self, executor: &mut FakeExecutor) {
for node in self.0.raw_nodes().iter() {
executor.add_account_data(&node.weight.account_data);
}
}

pub fn execute(&self, executor: &mut FakeExecutor) {
// Generate a list of modules
let accounts = toposort(&self.0, None).expect("Dep graph should be acyclic");
let mut expected_values = HashMap::new();
let mut modules = vec![];
for account_idx in accounts.iter().rev() {
let node = self.0.node_weight(*account_idx).expect("Node should exist");
let mut result = node.self_value;
let mut deps = vec![];

// Calculate the expected result of module entry function
for successor in self.0.neighbors_directed(*account_idx, Direction::Outgoing) {
result += expected_values
.get(&successor)
.expect("Topo sort should already compute the value for successor");
deps.push(self.0.node_weight(successor).unwrap().name.clone());
}
expected_values.insert(*account_idx, result);

let module_str =
module_generator::generate_module(&node.name, &deps, node.self_value, result);
let module = Compiler {
deps: modules.iter().collect(),
}
.into_compiled_module(&module_str)
.expect("Module compilation failed");

// Publish Module into executor.
let mut module_bytes = vec![];
module.serialize(&mut module_bytes).unwrap();

// TODO: Generate transactions instead of using add_module api.
executor.add_module(&node.name, module_bytes);

modules.push(module);
}
for account_idx in accounts.iter() {
let node = self.0.node_weight(*account_idx).expect("Node should exist");

// TODO: Generate transactions instead of using exec api.
executor.exec_module(&node.name, "foo", vec![], vec![]);
}
}
}
48 changes: 48 additions & 0 deletions aptos-move/e2e-tests/src/loader/module_generator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright © Aptos Foundation
// Parts of the project are originally copyright © Meta Platforms, Inc.
// SPDX-License-Identifier: Apache-2.0

//! Logic for account universes. This is not in the parent module to enforce privacy.

use move_core_types::language_storage::ModuleId;
use move_model::{code_writer::CodeWriter, emit, emitln, model::Loc};

// TODO: Use Move source language to generate the modules.
pub fn generate_module(
self_addr: &ModuleId,
deps: &[ModuleId],
self_val: u64,
expected_val: u64,
) -> String {
let writer = CodeWriter::new(Loc::default());
emit!(
writer,
"module {}.{} ",
self_addr.address(),
self_addr.name()
);
emitln!(writer, "{");
writer.indent();
for dep in deps {
emitln!(writer, "import {}.{};", dep.address(), dep.name());
}
emitln!(writer);
emitln!(writer, "public entry foo(): u64 {");
writer.indent();
emitln!(writer, "let a: u64;");
writer.unindent();
emitln!(writer, "label b0:");
writer.indent();
emit!(writer, "a = {}", self_val);
for dep in deps {
emit!(writer, "+ {}.foo()", dep.name());
}
emit!(writer, ";\n");
emitln!(writer, "assert(copy(a) == {}, 42);", expected_val);
emitln!(writer, "return move(a);");
writer.unindent();
emitln!(writer, "}");
writer.unindent();
emitln!(writer, "}");
writer.process_result(|s| s.to_string())
}
Loading

0 comments on commit ed3b35e

Please sign in to comment.