[verifier][adapter] Optional TxContext for entry points (MystenLabs#2219

)

* [verifier][adapter] Optional TxContext for entry points

- Made &mut TxContext optional for entry points
- Made it mandatory again for init functions, since its kind of useless to have an init function without it

crates/sui-adapter-transactional-tests/tests/entry_points/no_txn_context.exp

@@ -0,0 +1,15 @@
+processed 4 tasks
+
+init:
+A: object(100)
+
+task 1 'publish'. lines 6-24:
+created: object(104)
+written: object(103)
+
+task 2 'run'. lines 26-26:
+created: object(106)
+written: object(105)
+
+task 3 'run'. lines 28-28:
+written: object(106), object(107)

crates/sui-adapter-transactional-tests/tests/entry_points/no_txn_context.move

@@ -0,0 +1,28 @@
+// Copyright (c) 2022, Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+//# init --addresses Test=0x0 --accounts A
+
+//# publish
+module Test::M {
+    use Sui::TxContext::{Self, TxContext};
+    struct Obj has key {
+        id: Sui::ID::VersionedID,
+        value: u64
+    }
+
+    public(script) fun mint(ctx: &mut TxContext) {
+        Sui::Transfer::transfer(
+            Obj { id: TxContext::new_id(ctx), value: 0 },
+            TxContext::sender(ctx),
+        )
+    }
+
+    public(script) fun incr(obj: &mut Obj) {
+        obj.value = obj.value + 1
+    }
+}
+
+//# run Test::M::mint --sender A
+
+//# run Test::M::incr --sender A --args object(106)

crates/sui-adapter-transactional-tests/tests/publish/init_param.exp

@@ -1,4 +1,4 @@
 processed 2 tasks
 
 task 1 'publish'. lines 5-24:
-Error: Failed to verify the Move module, reason: "'init' function can have 0 or 1 parameter(s)".
+Error: Failed to verify the Move module, reason: "Expected exactly one parameter for _::M1::init  of type &mut Sui::TxContext::TxContext".

crates/sui-adapter/src/adapter.rs

@@ -23,7 +23,7 @@ use sui_types::{
     storage::{DeleteKind, Storage},
 };
 use sui_verifier::{
-    entry_points_verifier::{INIT_FN_NAME, RESOLVED_STD_OPTION, RESOLVED_SUI_ID},
+    entry_points_verifier::{is_tx_context, INIT_FN_NAME, RESOLVED_STD_OPTION, RESOLVED_SUI_ID},
     verifier,
 };
 
@@ -80,21 +80,24 @@ pub fn execute<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error =
     let is_genesis = ctx.digest() == TransactionDigest::genesis();
     let TypeCheckSuccess {
         module_id,
-        args,
+        mut args,
         object_data,
         by_value_objects,
         mutable_ref_objects,
+        has_ctx_arg,
     } = resolve_and_type_check(&objects, &module, function, &type_args, args, is_genesis)?;
 
-    let mut args = args;
-    args.push(ctx.to_vec());
+    if has_ctx_arg {
+        args.push(ctx.to_vec());
+    }
     execute_internal(
         vm,
         state_view,
         &module_id,
         function,
         type_args,
         args,
+        has_ctx_arg,
         object_data,
         by_value_objects,
         mutable_ref_objects,
@@ -116,6 +119,7 @@ fn execute_internal<
     function: &Identifier,
     type_args: Vec<TypeTag>,
     args: Vec<Vec<u8>>,
+    has_ctx_arg: bool,
     object_data: BTreeMap<ObjectID, (object::Owner, SequenceNumber)>,
     by_value_objects: BTreeSet<ObjectID>,
     mut mutable_ref_objects: BTreeMap<LocalIndex, ObjectID>,
@@ -157,7 +161,12 @@ fn execute_internal<
             // 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_reference_outputs.len());
+            let num_mut_objects = if has_ctx_arg {
+                mutable_ref_objects.len() + 1
+            } else {
+                mutable_ref_objects.len()
+            };
+            debug_assert!(num_mut_objects == mutable_reference_outputs.len());
 
             // When this function is used during publishing, it
             // may be executed several times, with objects being
@@ -166,9 +175,11 @@ fn execute_internal<
             // reflects what happened each time we call into the
             // Move VM (e.g. to account for the number of created
             // objects).
-            let (_, ctx_bytes, _) = mutable_reference_outputs.pop().unwrap();
-            let updated_ctx: TxContext = bcs::from_bytes(&ctx_bytes).unwrap();
-            ctx.update_state(updated_ctx)?;
+            if has_ctx_arg {
+                let (_, ctx_bytes, _) = mutable_reference_outputs.pop().unwrap();
+                let updated_ctx: TxContext = bcs::from_bytes(&ctx_bytes).unwrap();
+                ctx.update_state(updated_ctx)?;
+            }
 
             let mutable_refs = mutable_reference_outputs
                 .into_iter()
@@ -246,17 +257,12 @@ pub fn store_package_and_init_modules<
     let modules_to_init = modules
         .iter()
         .filter_map(|module| {
-            let init_fdef = module.function_defs.iter().find(|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))
+            Some(module.self_id())
         })
         .collect();
 
@@ -273,17 +279,14 @@ 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, /* needs TxContext */ bool)>,
+    module_ids_to_init: Vec<ModuleId>,
     ctx: &mut TxContext,
     gas_status: &mut SuiGasStatus,
 ) -> SuiResult {
     let init_ident = Identifier::new(INIT_FN_NAME.as_str()).unwrap();
-    for (module_id, needs_tx_context) in module_ids_to_init {
-        let args = if needs_tx_context {
-            vec![ctx.to_vec()]
-        } else {
-            vec![]
-        };
+    for module_id in module_ids_to_init {
+        let args = vec![ctx.to_vec()];
+        let has_ctx_arg = true;
 
         execute_internal(
             vm,
@@ -292,6 +295,7 @@ fn init_modules<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error
             &init_ident,
             Vec::new(),
             args,
+            has_ctx_arg,
             BTreeMap::new(),
             BTreeSet::new(),
             BTreeMap::new(),
@@ -646,6 +650,8 @@ pub struct TypeCheckSuccess {
     pub by_value_objects: BTreeSet<ObjectID>,
     pub mutable_ref_objects: BTreeMap<LocalIndex, ObjectID>,
     pub args: Vec<Vec<u8>>,
+    /// is TxContext included in the arguments?
+    pub has_ctx_arg: bool,
 }
 
 /// - Check that `package_object`, `module` and `function` are valid
@@ -661,6 +667,7 @@ pub fn resolve_and_type_check(
     is_genesis: bool,
 ) -> Result<TypeCheckSuccess, SuiError> {
     // Resolve the function we are calling
+    let view = &BinaryIndexedView::Module(module);
     let function_str = function.as_ident_str();
     let module_id = module.self_id();
     let fdef_opt = module.function_defs.iter().find(|fdef| {
@@ -701,8 +708,16 @@ pub fn resolve_and_type_check(
     }
 
     // total number of args is (|objects| + |pure_args|) + 1 for the the `TxContext` object
-    let num_args = args.len() + 1;
     let parameters = &module.signature_at(fhandle.parameters).0;
+    let has_ctx_arg = parameters
+        .last()
+        .map(|t| is_tx_context(view, t))
+        .unwrap_or(false);
+    let num_args = if has_ctx_arg {
+        args.len() + 1
+    } else {
+        args.len()
+    };
     if parameters.len() != num_args {
         return Err(SuiError::InvalidFunctionSignature {
             error: format!(
@@ -722,7 +737,6 @@ pub fn resolve_and_type_check(
     // Track the mapping from each input object to its Move type.
     // This will be needed latter in `check_child_object_of_shared_object`.
     let mut object_type_map = BTreeMap::new();
-    let view = &BinaryIndexedView::Module(module);
     let bcs_args = args
         .into_iter()
         .enumerate()
@@ -843,6 +857,7 @@ pub fn resolve_and_type_check(
         by_value_objects,
         mutable_ref_objects,
         args: bcs_args,
+        has_ctx_arg,
     })
 }
 

crates/sui-verifier-transactional-tests/tests/entry_points/optional_txn_context.exp

@@ -0,0 +1,9 @@
+processed 2 tasks
+
+task 0 'publish'. lines 4-11:
+created: object(103)
+written: object(102)
+
+task 1 'publish'. lines 14-23:
+created: object(105)
+written: object(104)

crates/sui-verifier-transactional-tests/tests/entry_points/optional_txn_context.mvir

@@ -0,0 +1,23 @@
+// Copyright (c) 2022, Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+//# publish
+module 0x0.M {
+    import 0x2.TxContext;
+    public(script) t() {
+        label l0:
+        abort 0;
+    }
+}
+
+
+//# publish
+module 0x0.M {
+    import 0x2.TxContext;
+    import 0x2.ID;
+    struct Obj has key { id: ID.VersionedID }
+    public(script) t(flag: bool, arg: &mut Self.Obj) {
+        label l0:
+        abort 0;
+    }
+}

crates/sui-verifier-transactional-tests/tests/init/must_have_txn_context.exp

@@ -0,0 +1,7 @@
+processed 2 tasks
+
+task 0 'publish'. lines 4-11:
+Error: Failed to verify the Move module, reason: "Expected exactly one parameter for _::M::init  of type &mut Sui::TxContext::TxContext".
+
+task 1 'publish'. lines 14-21:
+Error: Failed to verify the Move module, reason: "Expected exactly one parameter for _::M::init  of type &mut Sui::TxContext::TxContext".

crates/sui-verifier-transactional-tests/tests/init/must_have_txn_context.mvir

@@ -0,0 +1,21 @@
+// Copyright (c) 2022, Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+//# publish
+module 0x0.M {
+    import 0x2.TxContext;
+    init() {
+        label l0:
+        abort 0;
+    }
+}
+
+
+//# publish
+module 0x0.M {
+    import 0x2.TxContext;
+    init(ctx: &mut TxContext.TxContext, ctx2: &mut TxContext.TxContext) {
+        label l0:
+        abort 0;
+    }
+}

crates/sui-verifier/src/entry_points_verifier.rs

@@ -127,27 +127,29 @@ fn verify_init_function(module: &CompiledModule, fdef: &FunctionDefinition) -> R
     }
 
     let parameters = &view.signature_at(fhandle.parameters).0;
-    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(view, &parameters[0]),
-                ))
-            }
-        }
-        _ => Err(format!(
-            "'{}' function can have 0 or 1 parameter(s)",
-            INIT_FN_NAME
-        )),
+    if parameters.len() != 1 {
+        return Err(format!(
+            "Expected exactly one parameter for {}::{}  of type &mut {}::{}::{}",
+            module.self_id(),
+            INIT_FN_NAME,
+            SUI_FRAMEWORK_ADDRESS,
+            TX_CONTEXT_MODULE_NAME,
+            TX_CONTEXT_STRUCT_NAME,
+        ));
+    }
+
+    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(view, &parameters[0]),
+        ))
     }
 }
 
@@ -159,30 +161,11 @@ fn verify_entry_function_impl(
     let handle = view.function_handle_at(func_def.function);
     let params = view.signature_at(handle.parameters);
 
-    // must have at least on &mut TxContext param
-    if params.is_empty() {
-        return Err(format!(
-            "No parameters in entry function {}",
-            view.identifier_at(handle.name)
-        ));
-    }
-
-    let last_param = params.0.last().unwrap();
-    let last_param_idx = params.0.len() - 1;
-    if !is_tx_context(view, last_param) {
-        return Err(format!(
-            "{}::{}. Expected last parameter of function signature to be &mut {}::{}::{}, but \
-            found {}",
-            module.self_id(),
-            view.identifier_at(handle.name),
-            SUI_FRAMEWORK_ADDRESS,
-            TX_CONTEXT_MODULE_NAME,
-            TX_CONTEXT_STRUCT_NAME,
-            format_signature_token(view, last_param),
-        ));
-    }
-
-    for param in &params.0[0..last_param_idx] {
+    let all_non_ctx_params = match params.0.last() {
+        Some(last_param) if is_tx_context(view, last_param) => &params.0[0..params.0.len() - 1],
+        _ => &params.0,
+    };
+    for param in all_non_ctx_params {
         verify_param_type(view, &handle.type_parameters, param)?;
     }
 
@@ -257,7 +240,7 @@ fn is_primitive(
     }
 }
 
-fn is_tx_context(view: &BinaryIndexedView, p: &SignatureToken) -> bool {
+pub fn is_tx_context(view: &BinaryIndexedView, p: &SignatureToken) -> bool {
     match p {
         SignatureToken::MutableReference(m) => match &**m {
             SignatureToken::Struct(idx) => {