Fix a bug where transfer sui with insufficient gas still lead the obj…

…ect to be transferred (MystenLabs#3675)

* Add tests

* Fix insuffiicent gas bug

crates/sui-core/src/execution_engine.rs

@@ -238,6 +238,11 @@ fn execute_transaction<S: BackingPackageStore + ParentSync>(
         let cost_summary = gas_status.summary(result.is_ok());
         let gas_used = cost_summary.gas_used();
         let gas_rebate = cost_summary.storage_rebate;
+        // We must re-fetch the gas object from the temporary store, as it may have been reset
+        // previously in the case of error.
+        // TODO: It might be cleaner and less error-prone if we put gas object id into
+        // temporary store and move much of the gas logic there.
+        gas_object = temporary_store.read_object(&gas_object_id).unwrap().clone();
         gas::deduct_gas(&mut gas_object, gas_used, gas_rebate);
         trace!(gas_used, gas_obj_id =? gas_object.id(), gas_obj_ver =? gas_object.version(), "Updated gas object");
         temporary_store.write_object(gas_object);

crates/sui-core/src/unit_tests/gas_tests.rs

@@ -5,6 +5,7 @@ use super::*;
 
 use super::authority_tests::{init_state_with_ids, send_and_confirm_transaction};
 use super::move_integration_tests::build_and_try_publish_test_package;
+use crate::authority::authority_tests::init_state;
 use move_core_types::account_address::AccountAddress;
 use move_core_types::ident_str;
 use sui_adapter::genesis;
@@ -156,6 +157,39 @@ async fn test_native_transfer_gas_price_is_used() {
     );
 }
 
+#[tokio::test]
+async fn test_transfer_sui_insufficient_gas() {
+    let (sender, sender_key): (_, AccountKeyPair) = get_key_pair();
+    let recipient = dbg_addr(2);
+    let gas_object_id = ObjectID::random();
+    let gas_object = Object::with_id_owner_gas_for_testing(gas_object_id, sender, 50);
+    let gas_object_ref = gas_object.compute_object_reference();
+    let authority_state = init_state().await;
+    authority_state.insert_genesis_object(gas_object).await;
+
+    let kind = TransactionKind::Single(SingleTransactionKind::TransferSui(TransferSui {
+        recipient,
+        amount: None,
+    }));
+    let data = TransactionData::new_with_gas_price(kind, sender, gas_object_ref, 50, 1);
+    let signature = Signature::new(&data, &sender_key);
+    let tx = Transaction::new(data, signature);
+
+    let effects = send_and_confirm_transaction(&authority_state, tx)
+        .await
+        .unwrap()
+        .signed_effects
+        .unwrap()
+        .effects;
+    // We expect this to fail due to insufficient gas.
+    assert_eq!(
+        effects.status,
+        ExecutionStatus::new_failure(ExecutionFailureStatus::InsufficientGas)
+    );
+    // Ensure that the owner of the object did not change if the transfer failed.
+    assert_eq!(effects.mutated[0].1, sender);
+}
+
 #[tokio::test]
 async fn test_native_transfer_insufficient_gas_reading_objects() {
     // This test creates a transfer transaction with a gas budget, that's more than