[move-vm] Relax script function rules

- Scripts and public(script) functions can now take arguments of any type and return any values
- Checks are preserved for legacy file format versions
- Checks must be invoked by the adapter

Closes: #175

language/benchmarks/src/move_vm.rs

@@ -83,7 +83,13 @@ fn execute<M: Measurement + 'static>(
     c.bench_function(fun, |b| {
         b.iter(|| {
             session
-                .execute_function(&module_id, fun_name, vec![], vec![], &mut gas_status)
+                .execute_function_bypass_visibility(
+                    &module_id,
+                    fun_name,
+                    vec![],
+                    Vec::<Vec<u8>>::new(),
+                    &mut gas_status,
+                )
                 .unwrap_or_else(|err| {
                     panic!(
                         "{:?}::{} failed with {:?}",

language/extensions/async/move-async-vm/src/async_vm.rs

@@ -11,6 +11,7 @@ use move_binary_format::errors::{Location, PartialVMError, PartialVMResult, VMEr
 use move_core_types::{
     account_address::AccountAddress,
     effects::{ChangeSet, Event},
+    gas_schedule::GasAlgebra,
     identifier::Identifier,
     language_storage::{ModuleId, StructTag, TypeTag},
     resolver::MoveResolver,
@@ -19,7 +20,7 @@ use move_core_types::{
 use move_vm_runtime::{
     move_vm::MoveVM,
     native_functions::{NativeContextExtensions, NativeFunction},
-    session::{ExecutionResult, Session},
+    session::{SerializedReturnValues, Session},
 };
 use move_vm_types::{
     gas_schedule::GasStatus,
@@ -175,25 +176,29 @@ impl<'r, 'l, S: MoveResolver> AsyncSession<'r, 'l, S> {
         }
 
         // Execute the initializer.
-        let result = self.vm_session.execute_function_for_effects(
-            &actor.module_id,
-            &actor.initializer,
-            vec![],
-            Vec::<Vec<u8>>::new(),
-            gas_status,
-        );
+        let gas_before = gas_status.remaining_gas().get();
+        let result = self
+            .vm_session
+            .execute_function_bypass_visibility(
+                &actor.module_id,
+                &actor.initializer,
+                vec![],
+                Vec::<Vec<u8>>::new(),
+                gas_status,
+            )
+            .and_then(|ret| Ok((ret, self.vm_session.finish_with_extensions()?)));
+        let gas_used = gas_status.remaining_gas().get() - gas_before;
 
         // Process the result, moving the return value of the initializer function into the
         // changeset.
         match result {
-            ExecutionResult::Success {
-                mut change_set,
-                events,
-                mut return_values,
-                gas_used,
-                mut native_extensions,
-                ..
-            } => {
+            Ok((
+                SerializedReturnValues {
+                    mutable_reference_outputs: _,
+                    mut return_values,
+                },
+                (mut change_set, events, mut native_extensions),
+            )) => {
                 if return_values.len() != 1 {
                     Err(async_extension_error(format!(
                         "inconsistent initializer `{}`",
@@ -204,7 +209,7 @@ impl<'r, 'l, S: MoveResolver> AsyncSession<'r, 'l, S> {
                         &mut change_set,
                         actor_addr,
                         actor.state_tag.clone(),
-                        return_values.remove(0),
+                        return_values.remove(0).0,
                     )
                     .map_err(partial_vm_error_to_async)?;
                     let async_ext = native_extensions.remove::<AsyncExtension>();
@@ -216,7 +221,7 @@ impl<'r, 'l, S: MoveResolver> AsyncSession<'r, 'l, S> {
                     })
                 }
             }
-            ExecutionResult::Fail { error, gas_used } => Err(AsyncError { error, gas_used }),
+            Err(error) => Err(AsyncError { error, gas_used }),
         }
     }
 
@@ -266,37 +271,36 @@ impl<'r, 'l, S: MoveResolver> AsyncSession<'r, 'l, S> {
         );
 
         // Execute the handler.
-        let result = self.vm_session.execute_function_for_effects(
-            module_id,
-            handler_id,
-            vec![],
-            args,
-            gas_status,
-        );
+        let gas_before = gas_status.remaining_gas().get();
+        let result = self
+            .vm_session
+            .execute_function_bypass_visibility(module_id, handler_id, vec![], args, gas_status)
+            .and_then(|ret| Ok((ret, self.vm_session.finish_with_extensions()?)));
+
+        let gas_used = gas_status.remaining_gas().get() - gas_before;
 
         // Process the result, moving the mutated value of the handlers first parameter
         // into the changeset.
         match result {
-            ExecutionResult::Success {
-                mut change_set,
-                events,
-                mut mutable_ref_values,
-                gas_used,
-                mut native_extensions,
-                ..
-            } => {
-                if mutable_ref_values.len() > 1 {
+            Ok((
+                SerializedReturnValues {
+                    mut mutable_reference_outputs,
+                    return_values: _,
+                },
+                (mut change_set, events, mut native_extensions),
+            )) => {
+                if mutable_reference_outputs.len() > 1 {
                     Err(async_extension_error(format!(
                         "inconsistent handler `{}`",
                         handler_id
                     )))
                 } else {
-                    if !mutable_ref_values.is_empty() {
+                    if !mutable_reference_outputs.is_empty() {
                         publish_actor_state(
                             &mut change_set,
                             actor_addr,
                             actor.state_tag.clone(),
-                            mutable_ref_values.remove(0),
+                            mutable_reference_outputs.remove(0).1,
                         )
                         .map_err(partial_vm_error_to_async)?;
                     }
@@ -309,7 +313,7 @@ impl<'r, 'l, S: MoveResolver> AsyncSession<'r, 'l, S> {
                     })
                 }
             }
-            ExecutionResult::Fail { error, gas_used } => Err(AsyncError { error, gas_used }),
+            Err(error) => Err(AsyncError { error, gas_used }),
         }
     }
 

language/move-binary-format/src/check_bounds.rs

@@ -37,21 +37,30 @@ impl<'a> BoundsChecker<'a> {
         };
         bounds_check.verify_impl()?;
 
-        let signatures = &script.signatures;
-        let parameters = &script.parameters;
-        match signatures.get(parameters.into_index()) {
-            // The bounds checker has already checked each function definition's code, but a script's
-            // code exists outside of any function definition. It gets checked here.
-            Some(signature) => {
-                bounds_check.check_code(&script.code, &script.type_parameters, signature)
+        let type_param_count = script.type_parameters.len();
+
+        check_bounds_impl(bounds_check.view.signatures(), script.parameters)?;
+        if let Some(sig) = bounds_check
+            .view
+            .signatures()
+            .get(script.parameters.into_index())
+        {
+            for ty in &sig.0 {
+                bounds_check.check_type_parameter(ty, type_param_count)?
             }
-            None => Err(bounds_error(
-                StatusCode::INDEX_OUT_OF_BOUNDS,
-                IndexKind::Signature,
-                parameters.into_index() as u16,
-                signatures.len(),
-            )),
         }
+
+        // The bounds checker has already checked each function definition's code, but a
+        // script's code exists outside of any function definition. It gets checked here.
+        bounds_check.check_code(
+            &script.code,
+            &script.type_parameters,
+            bounds_check
+                .view
+                .signatures()
+                .get(script.parameters.into_index())
+                .unwrap(),
+        )
     }
 
     pub fn verify_module(module: &'a CompiledModule) -> PartialVMResult<()> {

language/move-binary-format/src/file_format_common.rs

@@ -374,8 +374,12 @@ pub const VERSION_3: u32 = 3;
 ///  + bytecode for vector operations
 pub const VERSION_4: u32 = 4;
 
+/// Version 5: changes compared with version 4
+///  +/- script and public(script) verification is now adapter specific
+pub const VERSION_5: u32 = 5;
+
 // Mark which version is the latest version
-pub const VERSION_MAX: u32 = VERSION_4;
+pub const VERSION_MAX: u32 = VERSION_5;
 
 pub(crate) mod versioned_data {
     use crate::{errors::*, file_format_common::*};

language/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs

@@ -70,6 +70,19 @@ fn invalid_type_param_in_fn_parameters() {
     );
 }
 
+#[test]
+fn invalid_type_param_in_script_parameters() {
+    use SignatureToken::*;
+
+    let mut s = basic_test_script();
+    s.parameters = SignatureIndex(1);
+    s.signatures.push(Signature(vec![TypeParameter(0)]));
+    assert_eq!(
+        BoundsChecker::verify_script(&s).unwrap_err().major_status(),
+        StatusCode::INDEX_OUT_OF_BOUNDS
+    );
+}
+
 #[test]
 fn invalid_struct_in_fn_return_() {
     use SignatureToken::*;

language/move-bytecode-verifier/src/lib.rs

@@ -25,6 +25,9 @@ pub mod verifier;
 pub use check_duplication::DuplicationChecker;
 pub use code_unit_verifier::CodeUnitVerifier;
 pub use instruction_consistency::InstructionConsistency;
+pub use script_signature::{
+    legacy_script_signature_checks, no_additional_script_signature_checks, FnCheckScriptSignature,
+};
 pub use signature::SignatureChecker;
 pub use struct_defs::RecursiveStructDefChecker;
 pub use verifier::{verify_module, verify_script};