Skip to content

Commit

Permalink
[Move adapter] Allow entry functions to return primitive values (Myst…
Browse files Browse the repository at this point in the history 
…enLabs#784)

* Early incomplete prototype and a test for allowing entry functions to return primitive values

* Updated rest server to use updated Move call return type

* Added support for vectors (up to second nesting level) as return types from entry functions

* Added required verfier modifications as well as multi-return test

* Updated result value to match the new execution status return type

* Addressed review comments

* Changed an explicit check for error into a more appropriate assertion

* Addressed more review comments
  • Loading branch information
@awelc
awelc authored Mar 15, 2022
1 parent 3afbc2f commit 9527322
Show file tree
Hide file tree
Showing 14 changed files with 490 additions and 44 deletions.
8 changes: 6 additions & 2 deletions sui/src/rest_server.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,9 @@ async fn transfer_object(
{
Ok((cert, effects)) => {
let gas_used = match effects.status {
ExecutionStatus::Success { gas_used } => gas_used,
// TODO: handle the actual return value stored in
// ExecutionStatus::Success
ExecutionStatus::Success { gas_used, .. } => gas_used,
ExecutionStatus::Failure { gas_used, error } => {
*server_context.wallet_context.lock().unwrap() = Some(wallet_context);
return Err(custom_http_error(
Expand Down Expand Up @@ -1244,7 +1246,9 @@ async fn handle_move_call(
{
Ok((cert, effects)) => {
let gas_used = match effects.status {
ExecutionStatus::Success { gas_used } => gas_used,
// TODO: handle the actual return value stored in
// ExecutionStatus::Success
ExecutionStatus::Success { gas_used, .. } => gas_used,
ExecutionStatus::Failure { gas_used, error } => {
let context = format!("Error calling move function, gas used {gas_used}");
return Err(anyhow::Error::new(error).context(context));
Expand Down
5 changes: 4 additions & 1 deletion sui_core/src/execution_engine.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,8 @@ fn transfer<S>(
});
}
temporary_store.write_object(output_object);
Ok(ExecutionStatus::Success { gas_used })
Ok(ExecutionStatus::Success {
gas_used,
results: vec![],
})
}
3 changes: 2 additions & 1 deletion sui_core/src/generate_format.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use sui_types::{
batch::UpdateItem,
crypto::{get_key_pair, AuthoritySignature, Signature},
error::SuiError,
messages::{ExecutionStatus, ObjectInfoRequestKind, TransactionKind},
messages::{CallResult, ExecutionStatus, ObjectInfoRequestKind, TransactionKind},
object::{Data, Owner},
serialize,
};
Expand Down Expand Up @@ -57,6 +57,7 @@ fn get_registry() -> Result<Registry> {
tracer.trace_type::<SuiError>(&samples)?;
tracer.trace_type::<Owner>(&samples)?;
tracer.trace_type::<ExecutionStatus>(&samples)?;
tracer.trace_type::<CallResult>(&samples)?;
tracer.trace_type::<Data>(&samples)?;
tracer.trace_type::<TypeTag>(&samples)?;
tracer.trace_type::<TypedStoreError>(&samples)?;
Expand Down
67 changes: 67 additions & 0 deletions sui_core/tests/staged/sui.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,70 @@ BatchInfoRequest:
BatchInfoResponseItem:
NEWTYPESTRUCT:
TYPENAME: UpdateItem
CallResult:
ENUM:
0:
Bool:
NEWTYPE: BOOL
1:
U8:
NEWTYPE: U8
2:
U64:
NEWTYPE: U64
3:
U128:
NEWTYPE: U128
4:
Address:
NEWTYPE:
TYPENAME: AccountAddress
5:
BoolVec:
NEWTYPE:
SEQ: BOOL
6:
U8Vec:
NEWTYPE:
SEQ: U8
7:
U64Vec:
NEWTYPE:
SEQ: U64
8:
U128Vec:
NEWTYPE:
SEQ: U128
9:
AddrVec:
NEWTYPE:
SEQ:
TYPENAME: AccountAddress
10:
BoolVecVec:
NEWTYPE:
SEQ: BOOL
11:
U8VecVec:
NEWTYPE:
SEQ:
SEQ: U8
12:
U64VecVec:
NEWTYPE:
SEQ:
SEQ: U64
13:
U128VecVec:
NEWTYPE:
SEQ:
SEQ: U128
14:
AddrVecVec:
NEWTYPE:
SEQ:
SEQ:
TYPENAME: AccountAddress
CertifiedTransaction:
STRUCT:
- transaction:
Expand Down Expand Up @@ -74,6 +138,9 @@ ExecutionStatus:
Success:
STRUCT:
- gas_used: U64
- results:
SEQ:
TYPENAME: CallResult
1:
Failure:
STRUCT:
Expand Down
95 changes: 86 additions & 9 deletions sui_programmability/adapter/src/adapter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
use anyhow::Result;

use crate::bytecode_rewriter::ModuleHandleRewriter;
use move_binary_format::{errors::PartialVMResult, file_format::CompiledModule};
use move_binary_format::{errors::PartialVMResult, file_format::CompiledModule, normalized::Type};
use sui_framework::EventType;
use sui_types::{
base_types::*,
error::{SuiError, SuiResult},
event::Event,
gas,
id::VersionedID,
messages::ExecutionStatus,
messages::{CallResult, ExecutionStatus},
move_package::*,
object::{MoveObject, Object, Owner},
storage::{DeleteKind, Storage},
Expand Down Expand Up @@ -91,6 +91,7 @@ pub fn execute<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error =
args,
mutable_ref_objects,
by_value_objects,
return_types,
} = match resolve_and_type_check(
package_object,
module,
Expand Down Expand Up @@ -119,15 +120,16 @@ pub fn execute<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error =
object_owner_map,
gas_budget,
ctx,
return_types,
) {
ExecutionStatus::Failure { gas_used, error } => {
exec_failure!(gas_used, *error)
}
ExecutionStatus::Success { gas_used } => {
ExecutionStatus::Success { gas_used, results } => {
match gas::try_deduct_gas(&mut gas_object, gas_used) {
Ok(()) => {
state_view.write_object(gas_object);
Ok(ExecutionStatus::Success { gas_used })
Ok(ExecutionStatus::Success { gas_used, results })
}
Err(err) => exec_failure!(gas_budget, err),
}
Expand All @@ -154,6 +156,7 @@ fn execute_internal<
object_owner_map: HashMap<SuiAddress, SuiAddress>,
gas_budget: u64, // gas budget for the current call operation
ctx: &mut TxContext,
return_types: Vec<Type>,
) -> ExecutionStatus {
// TODO: Update Move gas constants to reflect the gas fee on sui.
let cost_table = &move_vm_types::gas_schedule::INITIAL_COST_SCHEDULE;
Expand All @@ -179,15 +182,14 @@ fn execute_internal<
mut mutable_ref_values,
gas_used,
} => {
// we already checked that the function had no return types in resolve_and_type_check--it should
// also not return any values at runtime
debug_assert!(return_values.is_empty());
// Sui Move programs should never touch global state, so ChangeSet should be empty
debug_assert!(change_set.accounts().is_empty());
// Input ref parameters we put in should be the same number we get out, plus one for the &mut TxContext
debug_assert!(mutable_ref_objects.len() + 1 == mutable_ref_values.len());
debug_assert!(gas_used <= gas_budget);

let return_values = process_return_values(&return_values, &return_types);

// When this function is used during publishing, it
// may be executed several times, with objects being
// created in the Move VM in each Move call. In such
Expand Down Expand Up @@ -233,6 +235,7 @@ fn execute_internal<
} else {
ExecutionStatus::Success {
gas_used: total_gas,
results: return_values,
}
}
}
Expand All @@ -246,6 +249,77 @@ fn execute_internal<
}
}

fn process_return_values(values: &[Vec<u8>], return_types: &[Type]) -> Vec<CallResult> {
let mut results = vec![];
debug_assert!(values.len() == return_types.len());

for (idx, r) in return_types.iter().enumerate() {
match r {
// debug_assert-s for missing arms should be OK here as we
// already checked in
// MovePackage::check_and_get_entry_function that no other
// types can exist in the signature

// see CallResults struct comments for why this is
// implemented the way it is
Type::Bool => results.push(CallResult::Bool(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::U8 => results.push(CallResult::U8(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::U64 => results.push(CallResult::U64(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::U128 => results.push(CallResult::U128(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::Address => results.push(CallResult::Address(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::Vector(t) => match &**t {
Type::Bool => results.push(CallResult::BoolVec(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::U8 => results.push(CallResult::U8Vec(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::U64 => results.push(CallResult::U64Vec(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::U128 => results.push(CallResult::U128Vec(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::Address => results.push(CallResult::AddrVec(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::Vector(inner_t) => match &**inner_t {
Type::Bool => results.push(CallResult::BoolVecVec(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::U8 => results.push(CallResult::U8VecVec(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::U64 => results.push(CallResult::U64VecVec(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::U128 => results.push(CallResult::U128VecVec(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
Type::Address => results.push(CallResult::AddrVecVec(
bcs::from_bytes(values.get(idx).unwrap()).unwrap(),
)),
_ => debug_assert!(false),
},
_ => debug_assert!(false),
},
_ => debug_assert!(false),
}
}

results
}

/// Similar to execute(), only returns Err if there are system issues.
/// ExecutionStatus contains the actual execution result.
pub fn publish<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error = E> + Storage>(
Expand Down Expand Up @@ -310,7 +384,7 @@ pub fn publish<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error =
ctx,
gas_budget - gas_used_for_publish,
) {
ExecutionStatus::Success { gas_used } => gas_used,
ExecutionStatus::Success { gas_used, .. } => gas_used,
ExecutionStatus::Failure { gas_used, error } => {
// TODO: We should't charge the full publish cost when this failed.
// Instead we should only charge the cost to run bytecode verification.
Expand All @@ -326,6 +400,7 @@ pub fn publish<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error =
state_view.write_object(gas_object);
Ok(ExecutionStatus::Success {
gas_used: total_gas_used,
results: vec![],
})
}
Err(err) => exec_failure!(gas_budget, err),
Expand Down Expand Up @@ -385,8 +460,9 @@ fn init_modules<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error
HashMap::new(),
current_gas_budget,
ctx,
vec![], // no return types for module initializers
) {
ExecutionStatus::Success { gas_used } => gas_used,
ExecutionStatus::Success { gas_used, .. } => gas_used,
ExecutionStatus::Failure { gas_used, error } => {
return ExecutionStatus::Failure {
gas_used: gas_used + total_gas_used,
Expand All @@ -405,6 +481,7 @@ fn init_modules<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error

ExecutionStatus::Success {
gas_used: total_gas_used,
results: vec![],
}
}

Expand Down
Loading

0 comments on commit 9527322

Please sign in to comment.