Revert behavior to cost for entire contrat, fix test, add tests, fix …

…CHANGELOG

CHANGELOG.md

@@ -8,12 +8,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Changed
 
-- [#598](https://github.com/FuelLabs/fuel-vm/pull/598): Update cost model for `ldc` opcode to take into account contract size.
 - [#595](https://github.com/FuelLabs/fuel-vm/pull/595): Removed `wee_alloc` dependency from `fuel-asm`. It now uses the builtin allocator on web targets as well.
 
 
 #### Breaking
 
+- [#598](https://github.com/FuelLabs/fuel-vm/pull/598): Update cost model for `ldc` opcode to take into account contract size.
 - [#594](https://github.com/FuelLabs/fuel-vm/pull/594): Add new predicate input validation tests. Also improves error propagation so that predicate error message better reflects the reason for invalidity.
 - [#596](https://github.com/FuelLabs/fuel-vm/pull/596): Remove `core::ops::{Add, Sub}` impls from `BlockHeight`. Use `succ` and `pred` to access adjacent blocks, or perform arithmetic directly on the wrapped integer instead.
 

fuel-vm/src/interpreter/blockchain.rs

@@ -1,5 +1,3 @@
-use alloc::vec::Vec;
-
 use super::{
     contract::{
         balance,
@@ -57,6 +55,7 @@ use crate::{
         InterpreterStorage,
     },
 };
+use alloc::vec::Vec;
 use fuel_asm::PanicReason;
 use fuel_storage::StorageSize;
 use fuel_tx::{
@@ -101,7 +100,7 @@ where
         a: Word,
         b: Word,
         c: Word,
-    ) -> IoResult<(), S::DataError> {
+    ) -> IoResult<usize, S::DataError> {
         let contract_max_size = self.contract_max_size();
         let (
             SystemRegisters {
@@ -461,12 +460,13 @@ where
     /// contract_code = contracts[contract_id]
     /// mem[$ssp, $rC] = contract_code[$rB, $rC]
     /// ```
+    /// Returns the total length of the contract code that was loaded from storage.
     pub(crate) fn load_contract_code(
         mut self,
         contract_id_addr: Word,
         contract_offset: Word,
         length_unpadded: Word,
-    ) -> IoResult<(), S::DataError>
+    ) -> IoResult<usize, S::DataError>
     where
         I: Iterator<Item = &'vm ContractId>,
         S: InterpreterStorage,
@@ -498,14 +498,21 @@ where
 
         self.input_contracts.check(&contract_id)?;
 
+        let length_unpadded: usize = length_unpadded
+            .try_into()
+            .map_err(|_| PanicReason::MemoryOverflow)?;
+
         // Fetch the storage contract
-        // If $rC is greater than the code size, zero bytes are filled in.
-        let contract = super::contract::contract_subsection(
-            self.storage,
-            &contract_id,
-            length_unpadded as usize,
-        )?;
-        let contract = contract.as_ref();
+        let contract_bytes: Vec<u8> =
+            super::contract::contract(self.storage, &contract_id)?
+                .into_owned()
+                .into();
+        let contract_len = contract_bytes.len();
+        let start = contract_offset;
+        let finish = contract_offset
+            .checked_add(length_unpadded)
+            .ok_or(PanicReason::MemoryOverflow)?;
+        let contract_sub_bytes = &contract_bytes[start..finish];
 
         // Mark stack space as allocated
         let new_stack = dst_range.words().end;
@@ -515,7 +522,7 @@ where
         // Copy the code. Ownership checks are not used as the stack is adjusted above.
         copy_from_slice_zero_fill_noownerchecks(
             self.memory,
-            contract,
+            contract_sub_bytes,
             dst_range.start,
             contract_offset,
             length,
@@ -546,7 +553,7 @@ where
 
         inc_pc(self.pc)?;
 
-        Ok(())
+        Ok(contract_len)
     }
 }
 

fuel-vm/src/interpreter/contract.rs

@@ -154,26 +154,6 @@ where
         .ok_or_else(|| PanicReason::ContractNotFound.into())
 }
 
-/// Gets the bytes of the contract code from storage. If the contract is longer than
-/// specified `size` the contract will be shortened to `size` bytes. If the contract is
-/// shorter than `size` bytes, the contract will be padded with zeros.
-pub(crate) fn contract_subsection<S>(
-    storage: &S,
-    contract: &ContractId,
-    size: usize,
-) -> IoResult<Contract, S::DataError>
-where
-    S: InterpreterStorage,
-{
-    let mut buf = alloc::vec![0; size];
-    let _ = storage
-        .read_contract(contract, &mut buf)
-        .map_err(RuntimeError::Storage)?
-        .ok_or(PanicReason::ContractNotFound)?;
-    let contract = Contract::from(buf);
-    Ok(contract)
-}
-
 struct ContractBalanceCtx<'vm, S, I> {
     storage: &'vm S,
     memory: &'vm mut [u8; MEM_SIZE],

fuel-vm/src/interpreter/executors/instruction.rs

@@ -759,9 +759,9 @@ where
 
             Instruction::LDC(ldc) => {
                 let (a, b, c) = ldc.unpack();
-                let len = r!(c);
-                self.load_contract_code(r!(a), r!(b), len)?;
-                self.dependent_gas_charge(self.gas_costs().ldc, len)?;
+                // returns length of internal contract that was loaded
+                let len = self.load_contract_code(r!(a), r!(b), r!(c))?;
+                self.dependent_gas_charge(self.gas_costs().ldc, len as u64)?;
             }
 
             Instruction::LOG(log) => {

fuel-vm/src/tests/blockchain.rs

@@ -305,31 +305,65 @@ fn ldc__load_external_contract_code() {
 }
 
 #[test]
-fn ldc__gas_cost_is_dependent_on_size_of_rC() {
+fn ldc__gas_cost_is_not_dependent_on_rC() {
     let rng = &mut StdRng::seed_from_u64(2322u64);
     let salt: Salt = rng.gen();
 
     let mut client = MemoryClient::default();
 
     let gas_costs = client.gas_costs();
     let ldc_cost = gas_costs.ldc;
-    let bytes_per_gas_increase = ldc_cost.dep_per_unit;
+    let ldc_dep_cost = ldc_cost.dep_per_unit;
     let noop_cost = gas_costs.noop;
 
-    let starting_len = 0;
-    let starting_gas_amount = ldc__gas_cost_for_len(&mut client, rng, salt, starting_len);
+    let contract_size = 1000;
+    let starting_rC = 0;
+    let starting_gas_amount =
+        ldc__gas_cost_for_len(&mut client, rng, salt, contract_size, starting_rC);
 
     for i in 1..10 {
-        // Increase by bytes_per_gas_increase for each attempt
-        let len_diff = i * bytes_per_gas_increase as u16;
-        let len = starting_len + i * bytes_per_gas_increase as u16;
-        // The gas should go up 1 per bytes_per_gas_increase and noop_cost every 4 bytes
-        // (noop is 4 bytes)
-        let cost_of_copy = i as u64;
-        let cost_of_noops = len_diff as u64 / 4 * noop_cost;
-        let expected_gas_used = starting_gas_amount + cost_of_copy + cost_of_noops;
-
-        let actual_gas_used = ldc__gas_cost_for_len(&mut client, rng, salt, len);
+        // Increase by ldc_dep_cost for each attempt
+        let rC_diff = (i * ldc_dep_cost) as u16;
+        let rC = starting_rC + rC_diff;
+        // The gas should go up 0 per ldc_dep_cost and 1 per noop_cost every 4
+        // bytes (noop is 4 bytes)
+        let cost_of_noops = rC_diff as u64 / 4 * noop_cost;
+        let expected_gas_used = starting_gas_amount + cost_of_noops;
+
+        let actual_gas_used =
+            ldc__gas_cost_for_len(&mut client, rng, salt, contract_size, rC);
+        assert_eq!(actual_gas_used, expected_gas_used);
+    }
+}
+
+#[test]
+fn ldc__cost_is_proportional_to_total_contracts_size_not_rC() {
+    let rng = &mut StdRng::seed_from_u64(2322u64);
+    let salt: Salt = rng.gen();
+
+    let mut client = MemoryClient::default();
+
+    let gas_costs = client.gas_costs();
+    let ldc_cost = gas_costs.ldc;
+    let ldc_dep_cost = ldc_cost.dep_per_unit;
+
+    let contract_size = 0;
+    let rC = 0;
+    let starting_gas_amount =
+        ldc__gas_cost_for_len(&mut client, rng, salt, contract_size, rC);
+
+    let bytes_per_op = 4;
+
+    for i in 1..10 {
+        let contract_size = contract_size + (i * ldc_dep_cost / bytes_per_op) as usize;
+
+        let cost_of_ldc = i;
+
+        // The gas should go up 1 per every ldc_dep_cost even when rC is 0
+        let expected_gas_used = starting_gas_amount + cost_of_ldc;
+
+        let actual_gas_used =
+            ldc__gas_cost_for_len(&mut client, rng, salt, contract_size, rC);
         assert_eq!(actual_gas_used, expected_gas_used);
     }
 }
@@ -338,18 +372,19 @@ fn ldc__gas_cost_for_len(
     client: &mut MemoryClient,
     rng: &mut StdRng,
     salt: Salt,
-    len: u16,
+    // in number of opcodes
+    target_contract_size: usize,
+    rC: u16,
 ) -> Word {
     let mut target_contract = vec![];
-    for _ in 0..1000 {
+    for _ in 0..target_contract_size {
         target_contract.push(op::noop());
     }
 
     let bytes = target_contract.into_iter().collect::<Vec<u8>>();
     let contract_code: Witness = bytes.into();
 
-    let receipts =
-        ldc__load_len_of_target_contract(client, rng, salt, len, contract_code);
+    let receipts = ldc__load_len_of_target_contract(client, rng, salt, rC, contract_code);
 
     let result = receipts.last().expect("No receipt");