[move] scripts and entry functions should not return values (#11979)

aptos-labs · Mar 7, 2024 · c6a50c0 · c6a50c0
1 parent e3cdf20
commit c6a50c0
Show file tree

Hide file tree

Showing 12 changed files with 35 additions and 72 deletions.
diff --git a/aptos-move/aptos-transactional-test-harness/src/aptos_test_harness.rs b/aptos-move/aptos-transactional-test-harness/src/aptos_test_harness.rs
@@ -698,7 +698,7 @@ impl<'a> MoveTestAdapter<'a> for AptosTestAdapter<'a> {
         txn_args: Vec<MoveValue>,
         gas_budget: Option<u64>,
         extra_args: Self::ExtraRunArgs,
-    ) -> Result<(Option<String>, SerializedReturnValues)> {
+    ) -> Result<Option<String>> {
         let signer0 = self.compiled_state().resolve_address(&signers[0]);
 
         if gas_budget.is_some() {
@@ -763,14 +763,7 @@ impl<'a> MoveTestAdapter<'a> for AptosTestAdapter<'a> {
         } else {
             None
         };
-
-        //TODO: replace this dummy value with actual txn return value
-        let a = SerializedReturnValues {
-            mutable_reference_outputs: vec![(0, vec![0], MoveTypeLayout::U8)],
-            return_values: vec![(vec![0], MoveTypeLayout::U8)],
-        };
-
-        Ok((output, a))
+        Ok(output)
     }
 
     fn call_function(

diff --git a/aptos-move/aptos-transactional-test-harness/tests/aptos_test_harness/execute_script.exp b/aptos-move/aptos-transactional-test-harness/tests/aptos_test_harness/execute_script.exp
@@ -1,9 +1,5 @@
 processed 3 tasks
 
-task 1 'run'. lines 5-13:
-mutable inputs after call: local#0: 0
-return values: 0
-
 task 2 'view'. lines 15-15:
 key 0x1::coin::CoinStore<0x1::aptos_coin::AptosCoin> {
     coin: store 0x1::coin::Coin<0x1::aptos_coin::AptosCoin> {

diff --git a/aptos-move/aptos-vm-profiling/src/bins/run_move.rs b/aptos-move/aptos-vm-profiling/src/bins/run_move.rs
@@ -167,8 +167,7 @@ fn main() -> Result<()> {
     let src = fs::read_to_string(&args[1])?;
     if let Ok(script_blob) = Compiler::new(test_modules.iter().collect()).into_script_blob(&src) {
         let args: Vec<Vec<u8>> = vec![];
-        let res = sess.execute_script(script_blob, vec![], args, &mut UnmeteredGasMeter)?;
-        println!("{:?}", res);
+        sess.execute_script(script_blob, vec![], args, &mut UnmeteredGasMeter)?;
     } else {
         let module = Compiler::new(test_modules.iter().collect()).into_compiled_module(&src)?;
         let mut module_blob = vec![];

diff --git a/aptos-move/aptos-vm/src/aptos_vm.rs b/aptos-move/aptos-vm/src/aptos_vm.rs
@@ -92,7 +92,6 @@ use move_core_types::{
 use move_vm_runtime::{
     logging::expect_no_verification_errors,
     module_traversal::{TraversalContext, TraversalStorage},
-    session::SerializedReturnValues,
 };
 use move_vm_types::gas::{GasMeter, UnmeteredGasMeter};
 use num_cpus;
@@ -685,7 +684,7 @@ impl AptosVM {
         traversal_context: &mut TraversalContext,
         senders: Vec<AccountAddress>,
         script: &Script,
-    ) -> Result<SerializedReturnValues, VMStatus> {
+    ) -> Result<(), VMStatus> {
         // Note: Feature gating is needed here because the traversal of the dependencies could
         //       result in shallow-loading of the modules and therefore subtle changes in
         //       the error semantics.
@@ -713,7 +712,8 @@ impl AptosVM {
             self.features().is_enabled(FeatureFlag::STRUCT_CONSTRUCTORS),
         )?;
 
-        Ok(session.execute_script(script.code(), script.ty_args().to_vec(), args, gas_meter)?)
+        session.execute_script(script.code(), script.ty_args().to_vec(), args, gas_meter)?;
+        Ok(())
     }
 
     fn validate_and_execute_entry_function(
@@ -723,7 +723,7 @@ impl AptosVM {
         traversal_context: &mut TraversalContext,
         senders: Vec<AccountAddress>,
         entry_fn: &EntryFunction,
-    ) -> Result<SerializedReturnValues, VMStatus> {
+    ) -> Result<(), VMStatus> {
         // Note: Feature gating is needed here because the traversal of the dependencies could
         //       result in shallow-loading of the modules and therefore subtle changes in
         //       the error semantics.
@@ -758,13 +758,14 @@ impl AptosVM {
             &function,
             self.features().is_enabled(FeatureFlag::STRUCT_CONSTRUCTORS),
         )?;
-        Ok(session.execute_entry_function(
+        session.execute_entry_function(
             entry_fn.module(),
             entry_fn.function(),
             entry_fn.ty_args().to_vec(),
             args,
             gas_meter,
-        )?)
+        )?;
+        Ok(())
     }
 
     fn execute_script_or_entry_function<'a>(

diff --git a/aptos-move/writeset-transaction-generator/src/writeset_builder.rs b/aptos-move/writeset-transaction-generator/src/writeset_builder.rs
@@ -23,7 +23,6 @@ use move_core_types::{
     transaction_argument::convert_txn_args,
     value::{serialize_values, MoveValue},
 };
-use move_vm_runtime::session::SerializedReturnValues;
 use move_vm_types::gas::UnmeteredGasMeter;
 
 pub struct GenesisSession<'r, 'l>(SessionExt<'r, 'l>);
@@ -57,11 +56,7 @@ impl<'r, 'l> GenesisSession<'r, 'l> {
             });
     }
 
-    pub fn exec_script(
-        &mut self,
-        sender: AccountAddress,
-        script: &Script,
-    ) -> SerializedReturnValues {
+    pub fn exec_script(&mut self, sender: AccountAddress, script: &Script) {
         let mut temp = vec![sender.to_vec()];
         temp.extend(convert_txn_args(script.args()));
         self.0

diff --git a/third_party/move/move-vm/runtime/src/runtime.rs b/third_party/move/move-vm/runtime/src/runtime.rs
@@ -526,7 +526,7 @@ impl VMRuntime {
         module_store: &ModuleStorageAdapter,
         gas_meter: &mut impl GasMeter,
         extensions: &mut NativeContextExtensions,
-    ) -> VMResult<SerializedReturnValues> {
+    ) -> VMResult<()> {
         // load the script, perform verification
         let (
             func,
@@ -549,7 +549,8 @@ impl VMRuntime {
             module_store,
             gas_meter,
             extensions,
-        )
+        )?;
+        Ok(())
     }
 
     pub(crate) fn loader(&self) -> &Loader {

diff --git a/third_party/move/move-vm/runtime/src/session.rs b/third_party/move/move-vm/runtime/src/session.rs
@@ -82,7 +82,7 @@ impl<'r, 'l> Session<'r, 'l> {
         ty_args: Vec<TypeTag>,
         args: Vec<impl Borrow<[u8]>>,
         gas_meter: &mut impl GasMeter,
-    ) -> VMResult<SerializedReturnValues> {
+    ) -> VMResult<()> {
         let bypass_declared_entry_check = false;
         self.move_vm.runtime.execute_function(
             module,
@@ -94,7 +94,8 @@ impl<'r, 'l> Session<'r, 'l> {
             gas_meter,
             &mut self.native_extensions,
             bypass_declared_entry_check,
-        )
+        )?;
+        Ok(())
     }
 
     /// Similar to execute_entry_function, but it bypasses visibility checks
@@ -161,7 +162,7 @@ impl<'r, 'l> Session<'r, 'l> {
         ty_args: Vec<TypeTag>,
         args: Vec<impl Borrow<[u8]>>,
         gas_meter: &mut impl GasMeter,
-    ) -> VMResult<SerializedReturnValues> {
+    ) -> VMResult<()> {
         self.move_vm.runtime.execute_script(
             script,
             ty_args,

diff --git a/...d_party/move/move-vm/transactional-tests/tests/entry_points/modify_mutable_ref_inputs.exp b/...d_party/move/move-vm/transactional-tests/tests/entry_points/modify_mutable_ref_inputs.exp
@@ -1,7 +1,4 @@
 processed 3 tasks
 
-task 1 'run'. lines 28-40:
-mutable inputs after call: local#0: 112, local#1: { 224 }, local#2: [120, 0, 122]
-
 task 2 'run'. lines 42-42:
 mutable inputs after call: local#0: 112, local#1: { 224 }, local#2: [120, 0, 122]
diff --git a/third_party/move/move-vm/transactional-tests/tests/entry_points/ref_inputs.exp b/third_party/move/move-vm/transactional-tests/tests/entry_points/ref_inputs.exp
@@ -1,14 +1,5 @@
 processed 9 tasks
 
-task 2 'run'. lines 35-39:
-mutable inputs after call: local#0: false
-
-task 3 'run'. lines 41-46:
-mutable inputs after call: local#1: false, local#3: { 0 }
-
-task 4 'run'. lines 48-52:
-mutable inputs after call: local#1: false, local#3: { 0 }
-
 task 6 'run'. lines 56-56:
 mutable inputs after call: local#0: false
 

diff --git a/third_party/move/move-vm/transactional-tests/tests/entry_points/struct_arguments.exp b/third_party/move/move-vm/transactional-tests/tests/entry_points/struct_arguments.exp
@@ -1,11 +1,5 @@
 processed 5 tasks
 
-task 1 'run'. lines 19-24:
-mutable inputs after call: local#2: { 0 }
-
-task 2 'run'. lines 26-30:
-mutable inputs after call: local#2: { 0 }
-
 task 3 'run'. lines 32-32:
 mutable inputs after call: local#2: { 0 }
 

diff --git a/third_party/move/testing-infra/transactional-test-runner/src/framework.rs b/third_party/move/testing-infra/transactional-test-runner/src/framework.rs
@@ -150,7 +150,7 @@ pub trait MoveTestAdapter<'a>: Sized {
         args: Vec<<<Self as MoveTestAdapter<'a>>::ExtraValueArgs as ParsableValue>::ConcreteValue>,
         gas_budget: Option<u64>,
         extra: Self::ExtraRunArgs,
-    ) -> Result<(Option<String>, SerializedReturnValues)>;
+    ) -> Result<Option<String>>;
     fn call_function(
         &mut self,
         module: &ModuleId,
@@ -410,10 +410,8 @@ pub trait MoveTestAdapter<'a>: Sized {
                 };
                 let args = self.compiled_state().resolve_args(args)?;
                 let type_args = self.compiled_state().resolve_type_args(type_args)?;
-                let (mut output, return_values) =
+                let mut output =
                     self.execute_script(script, type_args, signers, args, gas_budget, extra_args)?;
-                let rendered_return_value = display_return_values(return_values);
-                output = merge_output(output, rendered_return_value);
                 if print_bytecode {
                     output = merge_output(output, printed);
                 }

diff --git a/third_party/move/testing-infra/transactional-test-runner/src/vm_test_harness.rs b/third_party/move/testing-infra/transactional-test-runner/src/vm_test_harness.rs
@@ -256,7 +256,7 @@ impl<'a> MoveTestAdapter<'a> for SimpleVMTestAdapter<'a> {
         txn_args: Vec<MoveValue>,
         gas_budget: Option<u64>,
         extra_args: Self::ExtraRunArgs,
-    ) -> Result<(Option<String>, SerializedReturnValues)> {
+    ) -> Result<Option<String>> {
         let signers: Vec<_> = signers
             .into_iter()
             .map(|addr| self.compiled_state().resolve_address(&addr))
@@ -276,24 +276,21 @@ impl<'a> MoveTestAdapter<'a> for SimpleVMTestAdapter<'a> {
             .chain(args)
             .collect();
         let verbose = extra_args.verbose;
-        let serialized_return_values = self
-            .perform_session_action(
-                gas_budget,
-                |session, gas_status| {
-                    session.execute_script(script_bytes, type_args, args, gas_status)
-                },
-                VMConfig::from(extra_args),
-            )
-            .map_err(|vm_error| {
-                anyhow!(
-                    "Script execution failed with VMError: {}",
-                    vm_error.format_test_output(
-                        move_test_debug() || verbose,
-                        !move_test_debug() && self.comparison_mode
-                    )
+        self.perform_session_action(
+            gas_budget,
+            |session, gas_status| session.execute_script(script_bytes, type_args, args, gas_status),
+            VMConfig::from(extra_args),
+        )
+        .map_err(|vm_error| {
+            anyhow!(
+                "Script execution failed with VMError: {}",
+                vm_error.format_test_output(
+                    move_test_debug() || verbose,
+                    !move_test_debug() && self.comparison_mode
                 )
-            })?;
-        Ok((None, serialized_return_values))
+            )
+        })?;
+        Ok(None)
     }
 
     fn call_function(