Skip to content

Commit

Permalink
[move][adapter] Static init rules (MystenLabs#1532)
Browse files Browse the repository at this point in the history
- Made init rules static
- The existence of the function is optional
- The function must be named 'init'
- The function must be private
- The function can have a single parameter, &mut TxContext
- Alternatively, the function can have zero parameters
  • Loading branch information
tnowacki authored Apr 22, 2022
1 parent fca9702 commit 637f67f
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 67 deletions.
37 changes: 24 additions & 13 deletions sui_programmability/adapter/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ use sui_types::{
object::{self, Data, MoveObject, Object, Owner},
storage::{DeleteKind, Storage},
};
use sui_verifier::{
entry_points_verifier::{self, INIT_FN_NAME},
verifier,
};
use sui_verifier::{entry_points_verifier::INIT_FN_NAME, verifier};

use move_core_types::{
account_address::AccountAddress,
Expand Down Expand Up @@ -244,12 +241,22 @@ pub fn store_package_and_init_modules<
ctx: &mut TxContext,
gas_status: &mut SuiGasStatus,
) -> SuiResult {
let mut modules_to_init = Vec::new();
for module in modules.iter() {
if entry_points_verifier::module_has_init(module) {
modules_to_init.push(module.self_id());
}
}
let modules_to_init = modules
.iter()
.filter_map(|module| {
let init_fdef = module.function_defs.iter().find(|fdef| {
let fhandle = module.function_handle_at(fdef.function).name;
let fname = module.identifier_at(fhandle);
fname == INIT_FN_NAME
})?;

let fhandle = module.function_handle_at(init_fdef.function);
let parameters = &module.signature_at(fhandle.parameters).0;
debug_assert!(parameters.len() <= 1);
let needs_tx_context = !parameters.is_empty();
Some((module.self_id(), needs_tx_context))
})
.collect();

// wrap the modules in an object, write it to the store
// The call to unwrap() will go away once we remove address owner from Immutable objects.
Expand All @@ -264,13 +271,17 @@ pub fn store_package_and_init_modules<
fn init_modules<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error = E> + Storage>(
state_view: &mut S,
vm: &MoveVM,
module_ids_to_init: Vec<ModuleId>,
module_ids_to_init: Vec<(ModuleId, /* needs TxContext */ bool)>,
ctx: &mut TxContext,
gas_status: &mut SuiGasStatus,
) -> SuiResult {
let init_ident = Identifier::new(INIT_FN_NAME.as_str()).unwrap();
for module_id in module_ids_to_init {
let args = vec![ctx.to_vec()];
for (module_id, needs_tx_context) in module_ids_to_init {
let args = if needs_tx_context {
vec![ctx.to_vec()]
} else {
vec![]
};

execute_internal(
vm,
Expand Down
36 changes: 20 additions & 16 deletions sui_programmability/adapter/src/unit_tests/adapter_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ fn publish_from_src(
src_path: &str,
gas_object: Object,
_gas_budget: u64,
) {
) -> SuiResult {
storage.write_object(gas_object);
storage.flush();

Expand All @@ -843,7 +843,6 @@ fn publish_from_src(
&mut tx_context,
&mut SuiGasStatus::new_unmetered(),
)
.unwrap();
}

#[test]
Expand All @@ -864,7 +863,8 @@ fn test_simple_call() {
"src/unit_tests/data/simple_call",
gas_object,
GAS_BUDGET,
);
)
.unwrap();
storage.flush();

// call published module function
Expand Down Expand Up @@ -921,7 +921,8 @@ fn test_publish_init() {
"src/unit_tests/data/publish_init",
gas_object,
GAS_BUDGET,
);
)
.unwrap();

// a package object and a fresh object in the constructor should
// have been crated
Expand Down Expand Up @@ -954,16 +955,17 @@ fn test_publish_init_public() {
Object::with_id_owner_for_testing(ObjectID::random(), base_types::SuiAddress::default());

// publish modules at a given path
publish_from_src(
assert!(publish_from_src(
&mut storage,
&native_functions,
"src/unit_tests/data/publish_init_public",
gas_object,
GAS_BUDGET,
);
)
.is_err());

// only a package object should have been crated
assert_eq!(storage.created().len(), 1);
// nothing should have been crated
assert_eq!(storage.created().len(), 0);
}

#[test]
Expand All @@ -980,16 +982,17 @@ fn test_publish_init_ret() {
Object::with_id_owner_for_testing(ObjectID::random(), base_types::SuiAddress::default());

// publish modules at a given path
publish_from_src(
assert!(publish_from_src(
&mut storage,
&native_functions,
"src/unit_tests/data/publish_init_ret",
gas_object,
GAS_BUDGET,
);
)
.is_err());

// only a package object should have been crated
assert_eq!(storage.created().len(), 1);
// nothing should have been crated
assert_eq!(storage.created().len(), 0);
}

#[test]
Expand All @@ -1006,14 +1009,15 @@ fn test_publish_init_param() {
Object::with_id_owner_for_testing(ObjectID::random(), base_types::SuiAddress::default());

// publish modules at a given path
publish_from_src(
assert!(publish_from_src(
&mut storage,
&native_functions,
"src/unit_tests/data/publish_init_param",
gas_object,
GAS_BUDGET,
);
)
.is_err());

// only a package object should have been crated
assert_eq!(storage.created().len(), 1);
// nothing should have been crated
assert_eq!(storage.created().len(), 0);
}
2 changes: 1 addition & 1 deletion sui_programmability/examples/nfts/sources/Num.move
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ module NFTs::Num {
const ETOO_MANY_NUMS: u64 = 0;

/// Create a unique issuer cap and give it to the transaction sender
public fun init(ctx: &mut TxContext) {
fun init(ctx: &mut TxContext) {
let issuer_cap = NumIssuerCap {
id: TxContext::new_id(ctx),
supply: 0,
Expand Down
97 changes: 60 additions & 37 deletions sui_programmability/verifier/src/entry_points_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,30 @@ use crate::format_signature_token;

pub const INIT_FN_NAME: &IdentStr = ident_str!("init");

/// Checks if parameters of functions that can become entry
/// functions (functions called directly from Sui) have correct types.
/// Checks valid rules rules for entry points, both for module initialization and transactions
///
/// We first identify functions that can be entry functions by looking
/// for functions with the following properties:
/// 1. Public
/// 2. Primitive return types (or vector of such up to 2 levels of nesting)
/// 3. Parameter order: objects, primitives, &mut TxContext
/// For module initialization
/// - The existence of the function is optional
/// - The function must have the name specified by `INIT_FN_NAME`
/// - The function must have `Visibility::Private`
/// - The function can have a single parameter: &mut TxContext (see `is_tx_context`)
/// - Alternatively, the function can have zero parameters
///
/// Note that this can be ambiguous in presence of the following
/// templated parameters:
/// - param: T
/// - param: vector<T> // and nested vectors
///
/// A function is considered an entry function if such templated
/// arguments are part of "object parameters" only.
///
/// In order for the parameter types of an entry function to be
/// correct, all generic types used in templated arguments mentioned
/// above must have the `key` ability.
/// For transaction entry points
/// - The function must have `Visibility::Script`
/// - The function must have at least one parameter: &mut TxContext (see `is_tx_context`)
/// - The transaction context parameter must be the last parameter
/// - The function cannot have any return values
pub fn verify_module(module: &CompiledModule) -> SuiResult {
for func_def in &module.function_defs {
let handle = module.function_handle_at(func_def.function);
let name = module.identifier_at(handle.name);
if name == INIT_FN_NAME {
verify_init_function(module, func_def)
.map_err(|error| SuiError::ModuleVerificationFailure { error })?;
continue;
}

// find candidate entry functions and checke their parameters
// (ignore other functions)
if func_def.visibility != Visibility::Script {
Expand All @@ -54,37 +56,57 @@ pub fn verify_module(module: &CompiledModule) -> SuiResult {
}

/// Checks if this module has a conformant `init`
// TODO make this static
pub fn module_has_init(module: &CompiledModule) -> bool {
fn verify_init_function(module: &CompiledModule, fdef: &FunctionDefinition) -> Result<(), String> {
let view = BinaryIndexedView::Module(module);
let fdef_opt = module.function_defs.iter().find(|fdef| {
let handle = view.function_handle_at(fdef.function);
let name = view.identifier_at(handle.name);
name == INIT_FN_NAME
});
let fdef = match fdef_opt {
None => return false,
Some(fdef) => fdef,
};

if fdef.visibility != Visibility::Private {
return false;
return Err(format!(
"{}. '{}' function cannot be public",
module.self_id(),
INIT_FN_NAME
));
}

let fhandle = module.function_handle_at(fdef.function);
if !fhandle.type_parameters.is_empty() {
return false;
return Err(format!(
"{}. '{}' function cannot have type parameters",
module.self_id(),
INIT_FN_NAME
));
}

if !view.signature_at(fhandle.return_).0.is_empty() {
return false;
return Err(format!(
"{}, '{}' function cannot have return values",
module.self_id(),
INIT_FN_NAME
));
}

let parameters = &view.signature_at(fhandle.parameters).0;
if parameters.len() != 1 {
return false;
match parameters.len() {
0 => Ok(()),
1 => {
if is_tx_context(&view, &parameters[0]) {
Ok(())
} else {
Err(format!(
"Expected parameter for {}::{} to be &mut mut {}::{}::{}, but found {}",
module.self_id(),
INIT_FN_NAME,
SUI_FRAMEWORK_ADDRESS,
TX_CONTEXT_MODULE_NAME,
TX_CONTEXT_STRUCT_NAME,
format_signature_token(module, &parameters[0]),
))
}
}
_ => Err(format!(
"'{}' function can have 0 or 1 parameter(s)",
INIT_FN_NAME
)),
}

is_tx_context(&view, &parameters[0])
}

fn verify_entry_function_impl(
Expand All @@ -105,7 +127,8 @@ fn verify_entry_function_impl(
let last_param = params.0.last().unwrap();
if !is_tx_context(&view, last_param) {
return Err(format!(
"{}::{}. Expected last parameter of function signature to be &mut {}::{}::{}, but found {}",
"{}::{}. Expected last parameter of function signature to be &mut {}::{}::{}, but \
found {}",
module.self_id(),
view.identifier_at(handle.name),
SUI_FRAMEWORK_ADDRESS,
Expand Down

0 comments on commit 637f67f

Please sign in to comment.