[cleanup] Remove unnecessary checks around TransferSui (MystenLabs#2841)

* [cleanup] Remove unnecessary checks around TransferSui

- Remove check for public transfer in TransferSui
- Fix gas object check around owned objects

crates/sui-adapter/src/adapter.rs

@@ -881,7 +881,7 @@ pub fn resolve_and_type_check(
                 t @ SignatureToken::Struct(_)
                 | t @ SignatureToken::StructInstantiation(_, _)
                 | t @ SignatureToken::TypeParameter(_) => {
-                    if !object.is_owned() {
+                    if !object.is_owned_or_quasi_shared() {
                         // Forbid passing shared (both mutable and immutable) object by value.
                         // This ensures that shared object cannot be transferred, deleted or wrapped.
                         return Err(ExecutionError::new_with_source(

crates/sui-core/src/authority.rs

@@ -823,7 +823,7 @@ impl AuthorityState {
             ObjectInfoRequestKind::LatestObjectInfo(request_layout) => {
                 match self.get_object(&request.object_id).await {
                     Ok(Some(object)) => {
-                        let lock = if !object.is_owned() {
+                        let lock = if !object.is_owned_or_quasi_shared() {
                             // Unowned obejcts have no locks.
                             None
                         } else {

crates/sui-core/src/authority/authority_store.rs

@@ -816,7 +816,7 @@ impl<S: Eq + Serialize + for<'de> Deserialize<'de>> SuiDataStore<S> {
 
         let owned_inputs: Vec<_> = active_inputs
             .iter()
-            .filter(|(id, _, _)| objects.get(id).unwrap().is_owned())
+            .filter(|(id, _, _)| objects.get(id).unwrap().is_owned_or_quasi_shared())
             .cloned()
             .collect();
 
@@ -921,7 +921,7 @@ impl<S: Eq + Serialize + for<'de> Deserialize<'de>> SuiDataStore<S> {
             let new_locks_to_init: Vec<_> = written
                 .iter()
                 .filter_map(|(_, (object_ref, new_object))| {
-                    if new_object.is_owned() {
+                    if new_object.is_owned_or_quasi_shared() {
                         Some(*object_ref)
                     } else {
                         None

crates/sui-core/src/execution_engine.rs

@@ -247,7 +247,8 @@ fn transfer_object<S>(
     sender: SuiAddress,
     recipient: SuiAddress,
 ) -> Result<(), ExecutionError> {
-    object.transfer_and_increment_version(recipient)?;
+    object.ensure_public_transfer_eligible()?;
+    object.transfer_and_increment_version(recipient);
     temporary_store.log_event(Event::TransferObject {
         package_id: ObjectID::from(SUI_FRAMEWORK_ADDRESS),
         transaction_module: Identifier::from(ident_str!("native")),
@@ -280,7 +281,8 @@ fn transfer_sui<S>(
 
     if let Some(amount) = amount {
         // Deduct the amount from the gas coin and update it.
-        let mut gas_coin = GasCoin::try_from(&object)?;
+        let mut gas_coin = GasCoin::try_from(&object)
+            .expect("gas object is transferred, so already checked to be a SUI coin");
         gas_coin.0.balance.withdraw(amount)?;
         let move_object = object
             .data
@@ -312,7 +314,7 @@ fn transfer_sui<S>(
     } else {
         // If amount is not specified, we simply transfer the entire coin object.
         // We don't want to increment the version number yet because latter gas charge will do it.
-        object.transfer_without_version_change(recipient)?;
+        object.transfer_without_version_change(recipient);
     }
 
     // TODO: Emit a new event type for this.

crates/sui-core/src/transaction_input_checker.rs

@@ -232,6 +232,7 @@ where
             }
         })
         .collect();
+
     for (object_kind, object) in input_objects.into_iter().zip(objects) {
         // All objects must exist in the DB.
         let object = match object {

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

@@ -461,6 +461,37 @@ async fn test_immutable_gas() {
     ));
 }
 
+// This test attempts to use an immutable gas object to pay for gas.
+// We expect it to fail early during transaction handle phase.
+#[tokio::test]
+async fn test_objected_owned_gas() {
+    let (sender, sender_key) = get_key_pair();
+    let recipient = dbg_addr(2);
+    let parent_object_id = ObjectID::random();
+    let authority_state = init_state_with_ids(vec![(sender, parent_object_id)]).await;
+    let child_object_id = ObjectID::random();
+    let child_object = Object::with_object_owner_for_testing(child_object_id, parent_object_id);
+    authority_state
+        .insert_genesis_object(child_object.clone())
+        .await;
+    let data = TransactionData::new_transfer_sui(
+        recipient,
+        sender,
+        None,
+        child_object.compute_object_reference(),
+        10000,
+    );
+    let signature = Signature::new(&data, &sender_key);
+    let transfer_transaction = Transaction::new(data, signature);
+    let result = authority_state
+        .handle_transaction(transfer_transaction.clone())
+        .await;
+    assert!(matches!(
+        result.unwrap_err(),
+        SuiError::InsufficientGas { .. }
+    ));
+}
+
 pub async fn send_and_confirm_transaction(
     authority: &AuthorityState,
     transaction: Transaction,

crates/sui-framework/src/natives/test_scenario.rs

@@ -197,7 +197,7 @@ fn get_inventory_for(
                     let obj_signer = obj.signer.unwrap();
                     obj.owner
                         == Owner::ObjectOwner(SuiAddress::try_from(parent.as_slice()).unwrap())
-                        && (!obj_signer.is_owned() || obj_signer == signer)
+                        && (!obj_signer.is_owned_or_quasi_shared() || obj_signer == signer)
                 } else {
                     obj.owner == signer
                 }

crates/sui-types/src/gas.rs

@@ -6,7 +6,7 @@ use crate::{
     error::{ExecutionError, ExecutionErrorKind},
     error::{SuiError, SuiResult},
     gas_coin::GasCoin,
-    object::Object,
+    object::{Object, Owner},
 };
 use move_core_types::{
     gas_schedule::{
@@ -301,13 +301,13 @@ impl<'a> SuiGasStatus<'a> {
 }
 
 /// Check whether the given gas_object and gas_budget is legit:
-/// 1. If the gas object is owned.
+/// 1. If the gas object has an address owner.
 /// 2. If it's enough to pay the flat minimum transaction fee
 /// 3. If it's less than the max gas budget allowed
 /// 4. If the gas_object actually has enough balance to pay for the budget.
 pub fn check_gas_balance(gas_object: &Object, gas_budget: u64, gas_price: u64) -> SuiResult {
     ok_or_gas_error!(
-        gas_object.is_owned(),
+        matches!(gas_object.owner, Owner::AddressOwner(_)),
         "Gas object must be owned Move object".to_owned()
     )?;
     ok_or_gas_error!(

crates/sui-types/src/object.rs

@@ -291,14 +291,16 @@ impl Owner {
     }
 
     pub fn is_immutable(&self) -> bool {
-        self == &Owner::Immutable
+        matches!(self, Owner::Immutable)
     }
 
-    pub fn is_owned(&self) -> bool {
-        match self {
-            Owner::AddressOwner(_) | Owner::ObjectOwner(_) => true,
-            Owner::Shared | Owner::Immutable => false,
-        }
+    /// Is owned by an address or an object
+    /// In the object case, it might be owned by an address owned object, a shared object, or
+    /// another object owned object.
+    /// If the root of this ownership is a shared object, we refer to these objects as
+    /// "quasi-shared", as they are not shared themselves, but behave similarly in some cases
+    pub fn is_owned_or_quasi_shared(&self) -> bool {
+        matches!(self, Owner::AddressOwner(_) | Owner::ObjectOwner(_))
     }
 
     pub fn is_shared(&self) -> bool {
@@ -388,8 +390,8 @@ impl Object {
         self.owner.is_immutable()
     }
 
-    pub fn is_owned(&self) -> bool {
-        self.owner.is_owned()
+    pub fn is_owned_or_quasi_shared(&self) -> bool {
+        self.owner.is_owned_or_quasi_shared()
     }
 
     pub fn is_shared(&self) -> bool {
@@ -460,25 +462,16 @@ impl Object {
 
     /// Change the owner of `self` to `new_owner`. This function does not increase the version
     /// number of the object.
-    pub fn transfer_without_version_change(
-        &mut self,
-        new_owner: SuiAddress,
-    ) -> Result<(), ExecutionError> {
-        self.ensure_public_transfer_eligible()?;
+    pub fn transfer_without_version_change(&mut self, new_owner: SuiAddress) {
         self.owner = Owner::AddressOwner(new_owner);
-        Ok(())
     }
 
     /// Change the owner of `self` to `new_owner`. This function will increment the version
     /// number of the object after transfer.
-    pub fn transfer_and_increment_version(
-        &mut self,
-        new_owner: SuiAddress,
-    ) -> Result<(), ExecutionError> {
-        self.transfer_without_version_change(new_owner)?;
+    pub fn transfer_and_increment_version(&mut self, new_owner: SuiAddress) {
+        self.transfer_without_version_change(new_owner);
         let data = self.data.try_as_move_mut().unwrap();
         data.increment_version();
-        Ok(())
     }
 
     pub fn immutable_with_id_for_testing(id: ObjectID) -> Self {
@@ -509,6 +502,20 @@ impl Object {
         }
     }
 
+    pub fn with_object_owner_for_testing(id: ObjectID, owner: ObjectID) -> Self {
+        let data = Data::Move(MoveObject {
+            type_: GasCoin::type_(),
+            has_public_transfer: true,
+            contents: GasCoin::new(id, SequenceNumber::new(), GAS_VALUE_FOR_TESTING).to_bcs_bytes(),
+        });
+        Self {
+            owner: Owner::ObjectOwner(owner.into()),
+            data,
+            previous_transaction: TransactionDigest::genesis(),
+            storage_rebate: 0,
+        }
+    }
+
     pub fn with_id_owner_for_testing(id: ObjectID, owner: SuiAddress) -> Self {
         // For testing, we provide sufficient gas by default.
         Self::with_id_owner_gas_for_testing(id, owner, GAS_VALUE_FOR_TESTING)