[script-composer] Add infer functionality, fix multiple return values

aptos-move/script-composer/src/builder.rs

@@ -186,25 +186,36 @@ impl TransactionComposer {
         self.modules.insert(module.self_id(), module);
     }
 
-    fn check_argument_compatibility(
+    fn infer_argument_compatibility(
         &mut self,
         argument: &AllocatedLocal,
         expected_ty: &SignatureToken,
-    ) -> anyhow::Result<()> {
+    ) -> anyhow::Result<AllocatedLocal> {
         let local_ty = if argument.is_parameter {
             self.parameters_ty[argument.local_idx as usize].clone()
         } else {
             self.locals_ty[argument.local_idx as usize].clone()
         };
 
-        let ty = match argument.op_type {
+        let mut inferred_op = argument.clone();
+        let local_ability = BinaryIndexedView::Script(self.builder.as_script())
+            .abilities(&local_ty, &[])
+            .map_err(|_| anyhow!("Failed to calculate ability for type"))?;
+
+        // For the copyable and droppable types, use copy instead of move by default to avoid explicit copy on
+        // the SDK side.
+        if local_ability.has_copy()
+            && local_ability.has_drop()
+            && inferred_op.op_type == ArgumentOperation::Move
+        {
+            inferred_op.op_type = ArgumentOperation::Copy;
+        };
+
+        let ty = match inferred_op.op_type {
             ArgumentOperation::Borrow => SignatureToken::Reference(Box::new(local_ty)),
             ArgumentOperation::BorrowMut => SignatureToken::MutableReference(Box::new(local_ty)),
             ArgumentOperation::Copy => {
-                let ability = BinaryIndexedView::Script(self.builder.as_script())
-                    .abilities(&local_ty, &[])
-                    .map_err(|_| anyhow!("Failed to calculate ability for type"))?;
-                if !ability.has_copy() {
+                if !local_ability.has_copy() {
                     bail!("Trying to copy move values that does NOT have copy ability");
                 }
                 local_ty
@@ -224,7 +235,7 @@ impl TransactionComposer {
         if &ty != expected_ty {
             bail!("Type mismatch when passing arugments around");
         }
-        Ok(())
+        Ok(inferred_op)
     }
 
     pub fn add_batched_call(
@@ -283,8 +294,8 @@ impl TransactionComposer {
                         is_parameter: false,
                         local_idx: self.calls[r.call_idx as usize].returns[r.return_idx as usize],
                     };
-                    self.check_argument_compatibility(&argument, &ty)?;
-                    arguments.push(argument)
+                    let inferred_argument = self.infer_argument_compatibility(&argument, &ty)?;
+                    arguments.push(inferred_argument)
                 },
                 CallArgument::Signer(i) => arguments.push(AllocatedLocal {
                     op_type: ArgumentOperation::Copy,
@@ -390,11 +401,11 @@ impl TransactionComposer {
             }
 
             // Storing return values
-            for arg in call.returns {
+            for arg in call.returns.iter().rev() {
                 script
                     .code
                     .code
-                    .push(Bytecode::StLoc((arg + parameters_count) as u8));
+                    .push(Bytecode::StLoc((*arg + parameters_count) as u8));
             }
         }
         script.code.code.push(Bytecode::Ret);

aptos-move/script-composer/src/tests/mod.rs

@@ -301,7 +301,9 @@ fn test_module() {
 
     run_txn(builder, &mut h);
 
-    // Create a copyable value and move it twice
+    // Create a droppable and copyable value and move it twice. This is ok because the builder
+    // will use copy instruction instead of move instruction for values that are both copyable
+    // and droppable.
     let mut builder = TransactionComposer::single_signer();
     load_module(&mut builder, &h, "0x1::batched_execution");
     let returns_1 = builder
@@ -329,7 +331,7 @@ fn test_module() {
         )
         .unwrap();
 
-    assert!(builder
+    builder
         .add_batched_call(
             "0x1::batched_execution".to_string(),
             "consume_copyable_value".to_string(),
@@ -339,6 +341,47 @@ fn test_module() {
                 CallArgument::new_bytes(MoveValue::U8(10).simple_serialize().unwrap()),
             ],
         )
+        .unwrap();
+
+    run_txn(builder, &mut h);
+
+    // Create a droppable value and move it twice
+    let mut builder = TransactionComposer::single_signer();
+    load_module(&mut builder, &h, "0x1::batched_execution");
+    let returns_1 = builder
+        .add_batched_call(
+            "0x1::batched_execution".to_string(),
+            "create_droppable_value".to_string(),
+            vec![],
+            vec![CallArgument::new_bytes(
+                MoveValue::U8(10).simple_serialize().unwrap(),
+            )],
+        )
+        .unwrap()
+        .pop()
+        .unwrap();
+
+    builder
+        .add_batched_call(
+            "0x1::batched_execution".to_string(),
+            "consume_droppable_value".to_string(),
+            vec![],
+            vec![
+                returns_1.clone(),
+                CallArgument::new_bytes(MoveValue::U8(10).simple_serialize().unwrap()),
+            ],
+        )
+        .unwrap();
+    assert!(builder
+        .add_batched_call(
+            "0x1::batched_execution".to_string(),
+            "consume_droppable_value".to_string(),
+            vec![],
+            vec![
+                returns_1,
+                CallArgument::new_bytes(MoveValue::U8(10).simple_serialize().unwrap()),
+            ],
+        )
         .is_err());
 
     // Copying a non-copyable value should return error on call.
@@ -635,4 +678,31 @@ fn test_module() {
             ],
         )
         .is_err());
+
+    // Test functions with multiple return values.
+    // Create a copyable value and copy it twice
+    let mut builder = TransactionComposer::single_signer();
+    load_module(&mut builder, &h, "0x1::batched_execution");
+    let returns = builder
+        .add_batched_call(
+            "0x1::batched_execution".to_string(),
+            "multiple_returns".to_string(),
+            vec![],
+            vec![],
+        )
+        .unwrap();
+
+    builder
+        .add_batched_call(
+            "0x1::batched_execution".to_string(),
+            "consume_non_droppable_value".to_string(),
+            vec![],
+            vec![
+                returns[1].clone(),
+                CallArgument::new_bytes(MoveValue::U8(1).simple_serialize().unwrap()),
+            ],
+        )
+        .unwrap();
+
+    run_txn(builder, &mut h);
 }

aptos-move/script-composer/src/tests/test_modules/sources/test.move

@@ -10,7 +10,7 @@ module 0x1::batched_execution {
         val: u8,
     }
 
-    struct CopyableValue has copy {
+    struct CopyableValue has drop, copy {
         val: u8,
     }
 
@@ -73,4 +73,8 @@ module 0x1::batched_execution {
         let GenericNonDroppableValue { val } = v;
         assert!(val == expected_val, 10);
     }
+
+    public fun multiple_returns(): (DroppableValue, NonDroppableValue) {
+        return (DroppableValue { val: 0 }, NonDroppableValue { val: 1} )
+    }
 }