Add more test cases for transfer and transfer_output (#535)

* Do small refactor, rename variables, fix bad test, add naive new test case

* Beef up TRO test and add internal case for TR test

* Update CHANGELOG

* Improve docs/param names

* Move CHANGELOG entry

* Remove debugging stuff, add sad cases

* Unify memory lookup helpers

* Simplify const construction

* Add tests that switch memory offsets

* Simplify helper functions with existing `read_bytes` function

* REmove helpers

* Add overflow tests

* Consolidate tests

CHANGELOG.md

@@ -10,6 +10,7 @@ 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`.
 - [#531](https://github.com/FuelLabs/fuel-vm/pull/531): UtxoId::from_str and TxPointer::from_str no longer crash on invalid input with multibyte characters. Also adds clippy lints to prevent future issues.
+- [#535](https://github.com/FuelLabs/fuel-vm/pull/535): Add better test coverage for TR and TRO
 
 #### Breaking
 

fuel-vm/src/interpreter/contract.rs

@@ -51,6 +51,7 @@ use fuel_types::{
     ContractId,
 };
 
+use crate::interpreter::memory::read_bytes;
 use std::borrow::Cow;
 
 #[cfg(test)]
@@ -201,37 +202,27 @@ struct TransferCtx<'vm, S, Tx> {
 }
 
 impl<'vm, S, Tx> TransferCtx<'vm, S, Tx> {
+    /// In Fuel specs:
+    /// Transfer $rB coins with asset ID at $rC to contract with ID at $rA.
+    /// $rA -> recipient_contract_id_offset
+    /// $rB -> transfer_amount
+    /// $rC -> asset_id_offset
     pub(crate) fn transfer(
         self,
         panic_context: &mut PanicContext,
-        a: Word,
-        b: Word,
-        c: Word,
+        recipient_contract_id_offset: Word,
+        transfer_amount: Word,
+        asset_id_offset: Word,
     ) -> Result<(), RuntimeError>
     where
         Tx: ExecutableTransaction,
         S: ContractsAssetsStorage,
         <S as StorageInspect<ContractsAssets>>::Error: Into<std::io::Error>,
     {
-        let ax = a
-            .checked_add(ContractId::LEN as Word)
-            .ok_or(PanicReason::ArithmeticOverflow)?;
-
-        let cx = c
-            .checked_add(AssetId::LEN as Word)
-            .ok_or(PanicReason::ArithmeticOverflow)?;
-
-        // if above usize::MAX then it cannot be safely cast to usize,
-        // check the tighter bound between VM_MAX_RAM and usize::MAX
-        if ax > MIN_VM_MAX_RAM_USIZE_MAX || cx > MIN_VM_MAX_RAM_USIZE_MAX {
-            return Err(PanicReason::MemoryOverflow.into())
-        }
-
-        let amount = b;
-        let destination = ContractId::try_from(&self.memory[a as usize..ax as usize])
-            .expect("Unreachable! Checked memory range");
-        let asset_id = AssetId::try_from(&self.memory[c as usize..cx as usize])
-            .expect("Unreachable! Checked memory range");
+        let amount = transfer_amount;
+        let destination =
+            ContractId::from(read_bytes(self.memory, recipient_contract_id_offset)?);
+        let asset_id = AssetId::from(read_bytes(self.memory, asset_id_offset)?);
 
         InputContracts::new(self.tx.input_contracts(), panic_context)
             .check(&destination)?;
@@ -282,38 +273,28 @@ impl<'vm, S, Tx> TransferCtx<'vm, S, Tx> {
         inc_pc(self.pc)
     }
 
+    /// In Fuel specs:
+    /// Transfer $rC coins with asset ID at $rD to address at $rA, with output $rB.
+    /// $rA -> recipient_offset
+    /// $rB -> output_index
+    /// $rC -> transfer_amount
+    /// $rD -> asset_id_offset
     pub(crate) fn transfer_output(
         self,
-        a: Word,
-        b: Word,
-        c: Word,
-        d: Word,
+        recipient_offset: Word,
+        output_index: Word,
+        transfer_amount: Word,
+        asset_id_offset: Word,
     ) -> Result<(), RuntimeError>
     where
         Tx: ExecutableTransaction,
         S: ContractsAssetsStorage,
         <S as StorageInspect<ContractsAssets>>::Error: Into<std::io::Error>,
     {
-        let ax = a
-            .checked_add(ContractId::LEN as Word)
-            .ok_or(PanicReason::ArithmeticOverflow)?;
-
-        let dx = d
-            .checked_add(AssetId::LEN as Word)
-            .ok_or(PanicReason::ArithmeticOverflow)?;
-
-        // if above usize::MAX then it cannot be safely cast to usize,
-        // check the tighter bound between VM_MAX_RAM and usize::MAX
-        if ax > MIN_VM_MAX_RAM_USIZE_MAX || dx > MIN_VM_MAX_RAM_USIZE_MAX {
-            return Err(PanicReason::MemoryOverflow.into())
-        }
-
-        let out_idx = b as usize;
-        let to = Address::try_from(&self.memory[a as usize..ax as usize])
-            .expect("Unreachable! Checked memory range");
-        let asset_id = AssetId::try_from(&self.memory[d as usize..dx as usize])
-            .expect("Unreachable! Checked memory range");
-        let amount = c;
+        let out_idx = output_index as usize;
+        let to = Address::from(read_bytes(self.memory, recipient_offset)?);
+        let asset_id = AssetId::from(read_bytes(self.memory, asset_id_offset)?);
+        let amount = transfer_amount;
 
         if amount == 0 {
             return Err(PanicReason::TransferZeroCoins.into())