Make TRO instruction revert on zero coin amount

CHANGELOG.md

@@ -10,12 +10,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - [#525](https://github.com/FuelLabs/fuel-vm/pull/525): The `$hp` register is no longer restored to it's previous value when returning from a call, making it possible to return heap-allocated types from `CALL`.
 
-
 #### Breaking
 
 - [#514](https://github.com/FuelLabs/fuel-vm/pull/514/): Add `ChainId` and `GasCosts` to `ConsensusParameters`. 
     Break down `ConsensusParameters` into sub-structs to match usage. Change signatures of functions to ask for
     necessary fields only.
+- [#532](https://github.com/FuelLabs/fuel-vm/pull/532): The `TRO` instruction now reverts when attempting to send zero coins to an output. Panic reason of this `TransferZeroCoins`, and `TR` was changed to use the same panic reason as well.
 
 ### Fixed
 

fuel-asm/src/panic_reason.rs

@@ -108,6 +108,8 @@ enum_from! {
         ArithmeticError = 0x23,
         /// The contract instruction is not allowed in predicates.
         ContractInstructionNotAllowed = 0x24,
+        /// Transfer of zero coins is not allowed.
+        TransferZeroCoins = 0x25,
     }
 }
 

fuel-vm/src/interpreter/contract.rs

@@ -237,7 +237,7 @@ impl<'vm, S, Tx> TransferCtx<'vm, S, Tx> {
             .check(&destination)?;
 
         if amount == 0 {
-            return Err(PanicReason::NotEnoughBalance.into())
+            return Err(PanicReason::TransferZeroCoins.into())
         }
 
         let internal_context = match internal_contract(self.context, self.fp, self.memory)
@@ -315,6 +315,10 @@ impl<'vm, S, Tx> TransferCtx<'vm, S, Tx> {
             .expect("Unreachable! Checked memory range");
         let amount = c;
 
+        if amount == 0 {
+            return Err(PanicReason::TransferZeroCoins.into())
+        }
+
         let internal_context = match internal_contract(self.context, self.fp, self.memory)
         {
             // optimistically attempt to load the internal contract id

fuel-vm/src/tests/outputs.rs

@@ -272,6 +272,116 @@ fn change_is_not_reduced_by_external_transfer_on_revert() {
     assert_eq!(change, input_amount);
 }
 
+#[test]
+fn zero_amount_transfer_reverts() {
+    let gas_price = 0;
+    let gas_limit = 1_000_000;
+    let asset_id = AssetId::default();
+
+    // setup state for test
+    // simple dummy contract for transferring value to
+    let contract_code = vec![op::ret(RegId::ONE)];
+
+    let mut test_context = TestBuilder::new(2322u64);
+    let contract_id = test_context
+        .setup_contract(contract_code, None, None)
+        .contract_id;
+
+    // setup script for transfer
+    let (script, _) = script_with_data_offset!(
+        data_offset,
+        vec![
+            // set reg 0x10 to contract id
+            op::movi(0x10, data_offset),
+            // set reg 0x12 to asset id
+            op::movi(0x11, data_offset + ContractId::LEN as u32),
+            // transfer to contract id at 0x10, amount of coins is zero, asset id at 0x11
+            op::tr(0x10, RegId::ZERO, 0x11),
+            op::ret(RegId::ONE),
+        ],
+        test_context.get_tx_params().tx_offset()
+    );
+
+    let script_data = [contract_id.as_ref(), asset_id.as_ref()]
+        .into_iter()
+        .flatten()
+        .copied()
+        .collect();
+
+    // execute and get receipts
+    let result = test_context
+        .start_script(script, script_data)
+        .gas_price(gas_price)
+        .gas_limit(gas_limit)
+        .coin_input(asset_id, 0)
+        .contract_input(contract_id)
+        .change_output(asset_id)
+        .contract_output(&contract_id)
+        .execute();
+
+    let receipts = result.receipts();
+
+    if let Some(Receipt::Panic { reason, .. }) = receipts.first() {
+        assert_eq!(reason.reason(), &PanicReason::TransferZeroCoins);
+    } else {
+        panic!("Expected a panic receipt");
+    }
+}
+
+#[test]
+fn zero_amount_transfer_out_reverts() {
+    let rng = &mut StdRng::seed_from_u64(2322u64);
+
+    // the initial external (coin) balance
+    let external_balance = 1_000_000;
+    // the amount to transfer out from external balance
+    let gas_price = 0;
+    let gas_limit = 1_000_000;
+    let asset_id = AssetId::default();
+    let owner: Address = rng.gen();
+
+    let (script, _) = script_with_data_offset!(
+        data_offset,
+        vec![
+            // load amount of coins to 0x10
+            op::movi(0x10, data_offset),
+            op::lw(0x10, 0x10, 0),
+            // load asset id to 0x11
+            op::movi(0x11, data_offset),
+            // load address to 0x12
+            op::movi(0x12, data_offset + 32),
+            // call contract without any tokens to transfer in or out
+            op::tro(0x12, RegId::ZERO, RegId::ZERO, 0x11),
+            op::ret(RegId::ONE),
+        ],
+        TxParameters::DEFAULT.tx_offset()
+    );
+
+    let script_data: Vec<u8> = [asset_id.as_ref(), owner.as_ref()]
+        .into_iter()
+        .flatten()
+        .copied()
+        .collect();
+
+    // execute and get receipts
+    let result = TestBuilder::new(2322u64)
+        .start_script(script, script_data)
+        .gas_price(gas_price)
+        .gas_limit(gas_limit)
+        .coin_input(asset_id, external_balance)
+        .variable_output(asset_id)
+        .change_output(asset_id)
+        .execute();
+
+    let receipts = result.receipts();
+
+    if let Some(Receipt::Panic { reason, .. }) = receipts.first() {
+        assert_eq!(reason.reason(), &PanicReason::TransferZeroCoins);
+    } else {
+        panic!("Expected a panic receipt");
+    }
+}
+
 #[test]
 fn variable_output_set_by_external_transfer_out() {
     let rng = &mut StdRng::seed_from_u64(2322u64);