Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[move-vm][closures] Type and Value representation and serialization #15670

Open
wants to merge 1 commit into
base: wrwg/clos_type_conv
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

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

5 changes: 5 additions & 0 deletions api/types/src/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,11 @@ impl<'a, S: StateView> MoveConverter<'a, S> {
MoveTypeLayout::Struct(struct_layout) => {
self.try_into_vm_value_struct(struct_layout, val)?
},
MoveTypeLayout::Function(..) => {
// TODO(#15664): do we actually need this? It appears the code here is dead and
// nowhere used
bail!("unexpected move type {:?} for value {:?}", layout, val)
},

// Some values, e.g., signer or ones with custom serialization
// (native), are not stored to storage and so we do not expect
Expand Down
8 changes: 8 additions & 0 deletions api/types/src/move_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,10 @@ impl From<TypeTag> for MoveType {
items: Box::new(MoveType::from(*v)),
},
TypeTag::Struct(v) => MoveType::Struct((*v).into()),
TypeTag::Function(..) => {
// TODO(#15664): determine whether functions and closures need to be supported
panic!("functions not supported by API types")
},
}
}
}
Expand All @@ -701,6 +705,10 @@ impl From<&TypeTag> for MoveType {
items: Box::new(MoveType::from(v.as_ref())),
},
TypeTag::Struct(v) => MoveType::Struct((&**v).into()),
TypeTag::Function(..) => {
// TODO(#15664): determine whether functions and closures need to be supported
panic!("functions not supported by API types")
},
}
}
}
Expand Down
26 changes: 26 additions & 0 deletions aptos-move/aptos-gas-schedule/src/gas_schedule/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,11 @@ where
self.offset = 1;
true
}

#[inline]
fn visit_closure(&mut self, depth: usize, len: usize) -> bool {
self.inner.visit_closure(depth, len)
}
}

struct AbstractValueSizeVisitor<'a> {
Expand Down Expand Up @@ -200,6 +205,13 @@ impl<'a> ValueVisitor for AbstractValueSizeVisitor<'a> {
true
}

#[inline]
fn visit_closure(&mut self, _depth: usize, _len: usize) -> bool {
// TODO(#15664): introduce a dedicated gas parameter?
self.size += self.params.struct_;
true
}

#[inline]
fn visit_vec(&mut self, _depth: usize, _len: usize) -> bool {
self.size += self.params.vector;
Expand Down Expand Up @@ -366,6 +378,13 @@ impl AbstractValueSizeGasParameters {
false
}

#[inline]
fn visit_closure(&mut self, _depth: usize, _len: usize) -> bool {
// TODO(#15664): independent gas parameter for closures?
self.res = Some(self.params.struct_);
false
}

#[inline]
fn visit_vec(&mut self, _depth: usize, _len: usize) -> bool {
self.res = Some(self.params.vector);
Expand Down Expand Up @@ -509,6 +528,13 @@ impl AbstractValueSizeGasParameters {
false
}

#[inline]
fn visit_closure(&mut self, _depth: usize, _len: usize) -> bool {
// TODO(#15664): independent gas parameter
self.res = Some(self.params.struct_);
false
}

#[inline]
fn visit_vec(&mut self, _depth: usize, _len: usize) -> bool {
self.res = Some(self.params.vector);
Expand Down
4 changes: 2 additions & 2 deletions aptos-move/aptos-sdk-builder/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ fn quote_type_as_format(type_tag: &TypeTag) -> Format {
tag if &**tag == Lazy::force(&str_tag) => Format::Seq(Box::new(Format::U8)),
_ => type_not_allowed(type_tag),
},
Signer => type_not_allowed(type_tag),
Signer | Function(..) => type_not_allowed(type_tag),
}
}

Expand Down Expand Up @@ -122,7 +122,7 @@ pub(crate) fn mangle_type(type_tag: &TypeTag) -> String {
tag if &**tag == Lazy::force(&str_tag) => "string".into(),
_ => type_not_allowed(type_tag),
},
Signer => type_not_allowed(type_tag),
Signer | Function(..) => type_not_allowed(type_tag),
}
}

Expand Down
8 changes: 4 additions & 4 deletions aptos-move/aptos-sdk-builder/src/golang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ func encode_{}_argument(arg {}) []byte {{
U8 => ("U8Vector", default_stmt),
_ => common::type_not_allowed(type_tag),
},
Struct(_) | Signer => common::type_not_allowed(type_tag),
Struct(_) | Signer | Function(..) => common::type_not_allowed(type_tag),
};
writeln!(
self.out,
Expand Down Expand Up @@ -793,7 +793,7 @@ func decode_{0}_argument(arg aptostypes.TransactionArgument) (value {1}, err err
tag if &**tag == Lazy::force(&str_tag) => "[]uint8".into(),
_ => common::type_not_allowed(type_tag),
},
Signer => common::type_not_allowed(type_tag),
Signer | Function(..) => common::type_not_allowed(type_tag),
}
}

Expand All @@ -820,7 +820,7 @@ func decode_{0}_argument(arg aptostypes.TransactionArgument) (value {1}, err err
U8 => format!("(*aptostypes.TransactionArgument__U8Vector)(&{})", name),
_ => common::type_not_allowed(type_tag),
},
Struct(_) | Signer => common::type_not_allowed(type_tag),
Struct(_) | Signer | Function(..) => common::type_not_allowed(type_tag),
}
}

Expand Down Expand Up @@ -854,7 +854,7 @@ func decode_{0}_argument(arg aptostypes.TransactionArgument) (value {1}, err err
tag if &**tag == Lazy::force(&str_tag) => Some("Bytes"),
_ => common::type_not_allowed(type_tag),
},
Signer => common::type_not_allowed(type_tag),
Signer | Function(..) => common::type_not_allowed(type_tag),
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions aptos-move/aptos-sdk-builder/src/rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@ static SCRIPT_FUNCTION_DECODER_MAP: once_cell::sync::Lazy<EntryFunctionDecoderMa
U8 => ("U8Vector", "Some(value)".to_string()),
_ => common::type_not_allowed(type_tag),
},
Struct(_) | Signer => common::type_not_allowed(type_tag),
Struct(_) | Signer | Function(..) => common::type_not_allowed(type_tag),
};
writeln!(
self.out,
Expand Down Expand Up @@ -880,7 +880,7 @@ fn decode_{}_argument(arg: TransactionArgument) -> Option<{}> {{
tag if &**tag == Lazy::force(&str_tag) => "Vec<u8>".into(),
_ => common::type_not_allowed(type_tag),
},
Signer => common::type_not_allowed(type_tag),
Signer | Function(..) => common::type_not_allowed(type_tag),
}
}

Expand Down Expand Up @@ -909,7 +909,7 @@ fn decode_{}_argument(arg: TransactionArgument) -> Option<{}> {{
_ => common::type_not_allowed(type_tag),
},

Struct(_) | Signer => common::type_not_allowed(type_tag),
Struct(_) | Signer | Function(..) => common::type_not_allowed(type_tag),
}
}
}
Expand Down
1 change: 1 addition & 0 deletions aptos-move/aptos-vm/src/move_vm_ext/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ impl<'r, 'l> SessionExt<'r, 'l> {
module_storage: &impl ModuleStorage,
) -> VMResult<(VMChangeSet, ModuleWriteSet)> {
let move_vm = self.inner.get_move_vm();

let function_extension = module_storage.as_function_value_extension();

let resource_converter = |value: Value,
Expand Down
10 changes: 7 additions & 3 deletions aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ pub(crate) fn is_valid_txn_arg(
let full_name = format!("{}::{}", st.module.short_str_lossless(), st.name);
allowed_structs.contains_key(&full_name)
}),
Signer | Reference(_) | MutableReference(_) | TyParam(_) => false,
Signer | Reference(_) | MutableReference(_) | TyParam(_) | Function { .. } => false,
}
}

Expand Down Expand Up @@ -300,7 +300,9 @@ fn construct_arg(
Err(invalid_signature())
}
},
Reference(_) | MutableReference(_) | TyParam(_) => Err(invalid_signature()),
Reference(_) | MutableReference(_) | TyParam(_) | Function { .. } => {
Err(invalid_signature())
},
}
}

Expand Down Expand Up @@ -369,7 +371,9 @@ pub(crate) fn recursively_construct_arg(
U64 => read_n_bytes(8, cursor, arg)?,
U128 => read_n_bytes(16, cursor, arg)?,
U256 | Address => read_n_bytes(32, cursor, arg)?,
Signer | Reference(_) | MutableReference(_) | TyParam(_) => return Err(invalid_signature()),
Signer | Reference(_) | MutableReference(_) | TyParam(_) | Function { .. } => {
return Err(invalid_signature())
},
};
Ok(())
}
Expand Down
5 changes: 3 additions & 2 deletions aptos-move/framework/move-stdlib/src/natives/bcs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,11 @@ fn constant_serialized_size(ty_layout: &MoveTypeLayout) -> (u64, PartialVMResult
MoveTypeLayout::Signer => Ok(None),
// vectors have no constant size
MoveTypeLayout::Vector(_) => Ok(None),
// enums have no constant size
// enums and functions have no constant size
MoveTypeLayout::Struct(
MoveStructLayout::RuntimeVariants(_) | MoveStructLayout::WithVariants(_),
) => Ok(None),
)
| MoveTypeLayout::Function(..) => Ok(None),
MoveTypeLayout::Struct(MoveStructLayout::Runtime(fields)) => {
let mut total = Some(0);
for field in fields {
Expand Down
19 changes: 18 additions & 1 deletion aptos-move/framework/src/natives/string_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use move_core_types::{
use move_vm_runtime::native_functions::NativeFunction;
use move_vm_types::{
loaded_data::runtime_types::Type,
values::{Reference, Struct, Value, Vector, VectorRef},
values::{Closure, Reference, Struct, Value, Vector, VectorRef},
};
use smallvec::{smallvec, SmallVec};
use std::{collections::VecDeque, fmt::Write, ops::Deref};
Expand Down Expand Up @@ -350,6 +350,23 @@ fn native_format_impl(
)?;
out.push('}');
},
MoveTypeLayout::Function(_) => {
let (fun, args) = val.value_as::<Closure>()?.unpack();
let data = context
.context
.function_value_extension()
.get_serialization_data(fun.as_ref())?;
out.push_str(&fun.to_stable_string());
format_vector(
context,
data.captured_layouts.iter(),
args.collect(),
depth,
!context.single_line,
out,
)?;
out.push(')');
},

// This is unreachable because we check layout at the start. Still, return
// an error to be safe.
Expand Down
15 changes: 11 additions & 4 deletions aptos-move/script-composer/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ pub(crate) fn import_type_tag(
type_tag: &TypeTag,
module_resolver: &BTreeMap<ModuleId, CompiledModule>,
) -> PartialVMResult<SignatureToken> {
let to_list = |script_builder: &mut CompiledScriptBuilder, ts: &[TypeTag]| {
ts.iter()
.map(|t| import_type_tag(script_builder, t, module_resolver))
.collect::<PartialVMResult<Vec<_>>>()
};
Ok(match type_tag {
TypeTag::Address => SignatureToken::Address,
TypeTag::U8 => SignatureToken::U8,
Expand All @@ -53,13 +58,15 @@ pub(crate) fn import_type_tag(
} else {
SignatureToken::StructInstantiation(
struct_idx,
s.type_args
.iter()
.map(|ty| import_type_tag(script_builder, ty, module_resolver))
.collect::<PartialVMResult<Vec<_>>>()?,
to_list(script_builder, &s.type_args)?,
)
}
},
TypeTag::Function(f) => SignatureToken::Function(
to_list(script_builder, &f.args)?,
to_list(script_builder, &f.results)?,
f.abilities,
),
})
}

Expand Down
8 changes: 7 additions & 1 deletion testsuite/fuzzer/fuzz/fuzz_targets/move/utils/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
use aptos_language_e2e_tests::{account::Account, executor::FakeExecutor};
use arbitrary::Arbitrary;
use move_binary_format::file_format::CompiledModule;
use move_core_types::value::{MoveStructLayout, MoveTypeLayout};
use move_core_types::{
function::MoveFunctionLayout,
value::{MoveStructLayout, MoveTypeLayout},
};

#[macro_export]
macro_rules! tdbg {
Expand Down Expand Up @@ -82,6 +85,9 @@ pub(crate) fn is_valid_layout(layout: &MoveTypeLayout) -> bool {
}
fields.iter().all(is_valid_layout)
},
L::Function(MoveFunctionLayout(args, results, _)) => {
args.iter().chain(results).all(is_valid_layout)
},
L::Struct(_) => {
// decorated layouts not supported
false
Expand Down
1 change: 1 addition & 0 deletions third_party/move/move-binary-format/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ fn construct_ty_for_constant(layout: &MoveTypeLayout) -> Option<SignatureToken>
construct_ty_for_constant(l.as_ref())?,
))),
MoveTypeLayout::Struct(_) => None,
MoveTypeLayout::Function(_) => None,
MoveTypeLayout::Bool => Some(SignatureToken::Bool),

// It is not possible to have native layout for constant values.
Expand Down
4 changes: 4 additions & 0 deletions third_party/move/move-binary-format/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,10 @@ impl PartialVMError {
}))
}

pub fn new_invariant_violation(msg: impl ToString) -> PartialVMError {
Self::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR).with_message(msg.to_string())
}

pub fn major_status(&self) -> StatusCode {
self.0.major_status
}
Expand Down
1 change: 1 addition & 0 deletions third_party/move/move-binary-format/src/normalized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ impl From<TypeTag> for Type {
name: s.name,
type_arguments: s.type_args.into_iter().map(|ty| ty.into()).collect(),
},
TypeTag::Function(_) => panic!("function types not supported in normalized types"),
}
}
}
Expand Down
1 change: 1 addition & 0 deletions third_party/move/move-compiler/src/cfgir/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ impl AstDebug for MoveValue {
},
V::Struct(_) => panic!("ICE struct constants not supported"),
V::Signer(_) => panic!("ICE signer constants not supported"),
V::Closure(_) => panic!("ICE closures not supported"),
}
}
}
Expand Down
Loading
Loading