Avoid conversion into usize type and use u32 or u64 instead (#789)

* Avoid conversion into `usize` type and use `u32` or `u64` instead * Use explicit conversion into `u32` * Don't affect the `StorageSize` trait * Updated CHANGELOG.md
FuelLabs · Jul 4, 2024 · a6abe47 · a6abe47
1 parent c329ea7
commit a6abe47
Show file tree

Hide file tree

Showing 16 changed files with 66 additions and 75 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 #### Breaking
+- [#789](https://github.com/FuelLabs/fuel-vm/pull/789): Avoid conversion into `usize` type and use `u32` or `u64` instead. The change is breaking since could return other errors for 32-bit systems.
 - [#786](https://github.com/FuelLabs/fuel-vm/pull/786): Fixed the CCP opcode to charge for the length from the input arguments.
 - [#785](https://github.com/FuelLabs/fuel-vm/pull/785): Require `ContractCreated` output in the `Create` transaction. The `TransactionBuilder<Create>` has a `add_contract_created` method to simplify the creation of the `ContractCreated` output for tests.
 

diff --git a/ci_checks.sh b/ci_checks.sh
@@ -3,7 +3,7 @@
 # The script runs almost all CI checks locally.
 #
 # Requires installed:
-# - Rust `1.72.0`
+# - Rust `1.75.0`
 # - Nightly rust formatter
 # - `rustup target add thumbv6m-none-eabi`
 # - `rustup target add wasm32-unknown-unknown`

diff --git a/fuel-tx/src/transaction/types/chargeable_transaction.rs b/fuel-tx/src/transaction/types/chargeable_transaction.rs
@@ -268,11 +268,7 @@ mod field {
                         self.inputs_offset_at(idx)
                             .map(|inputs| inputs.saturating_add(predicate))
                     })
-                    .zip(
-                        input
-                            .predicate_len()
-                            .map(|l| bytes::padded_len_usize(l).unwrap_or(usize::MAX)),
-                    )
+                    .zip(input.predicate_len().and_then(bytes::padded_len_usize))
             })
         }
     }

diff --git a/fuel-vm/Cargo.toml b/fuel-vm/Cargo.toml
@@ -46,7 +46,7 @@ serde_with = { version = "3.7", optional = true }
 sha3 = { version = "0.10", default-features = false }
 static_assertions = "1.1"
 strum = { version = "0.24", features = ["derive"], default-features = false }
-tai64 = { version = "4.0", default-features = false }
+tai64 = { version = "4.0", default-features = false, optional = true }
 
 [dev-dependencies]
 criterion = "0.4"
@@ -104,5 +104,6 @@ test-helpers = [
     "alloc",
     "random",
     "dep:anyhow",
+    "tai64",
     "fuel-crypto/test-helpers",
 ]
diff --git a/fuel-vm/src/call.rs b/fuel-vm/src/call.rs
@@ -71,7 +71,7 @@ pub struct CallFrame {
     to: ContractId,
     asset_id: AssetId,
     registers: [Word; VM_REGISTER_COUNT],
-    code_size: usize,
+    code_size_padded: usize,
     a: Word,
     b: Word,
 }
@@ -83,7 +83,7 @@ impl Default for CallFrame {
             to: ContractId::default(),
             asset_id: AssetId::default(),
             registers: [0; VM_REGISTER_COUNT],
-            code_size: 0,
+            code_size_padded: 0,
             a: 0,
             b: 0,
         }
@@ -92,22 +92,22 @@ impl Default for CallFrame {
 
 impl CallFrame {
     /// Create a new call frame.
-    pub const fn new(
+    pub fn new(
         to: ContractId,
         asset_id: AssetId,
         registers: [Word; VM_REGISTER_COUNT],
         code_size: usize,
         a: Word,
         b: Word,
-    ) -> Self {
-        Self {
+    ) -> Option<Self> {
+        Some(Self {
             to,
             asset_id,
             registers,
-            code_size,
+            code_size_padded: padded_len_usize(code_size)?,
             a,
             b,
-        }
+        })
     }
 
     /// Start of the contract id offset from the beginning of the call frame.
@@ -155,21 +155,10 @@ impl CallFrame {
         &self.to
     }
 
+    #[cfg(feature = "test-helpers")]
     /// Contract code length in bytes.
-    pub fn code_size(&self) -> usize {
-        self.code_size
-    }
-
-    /// Padding to the next word boundary.
-    pub fn code_size_padding(&self) -> usize {
-        self.total_code_size()
-            .checked_sub(self.code_size)
-            .expect("Padding is always less than size + padding")
-    }
-
-    /// Total code size including padding.
-    pub fn total_code_size(&self) -> usize {
-        padded_len_usize(self.code_size).expect("Code size cannot overflow with padding")
+    pub fn code_size_padded(&self) -> usize {
+        self.code_size_padded
     }
 
     /// `a` argument.

diff --git a/fuel-vm/src/interpreter/blockchain.rs b/fuel-vm/src/interpreter/blockchain.rs
@@ -57,7 +57,6 @@ use fuel_tx::{
     Receipt,
 };
 use fuel_types::{
-    bytes,
     bytes::padded_len_word,
     Address,
     AssetId,
@@ -588,19 +587,13 @@ where
         }
 
         let contract_id = ContractId::from(self.memory.read_bytes(contract_id_addr)?);
-        let contract_offset: usize = contract_offset
-            .try_into()
-            .map_err(|_| PanicReason::MemoryOverflow)?;
+        let contract_offset =
+            u32::try_from(contract_offset).map_err(|_| PanicReason::MemoryOverflow)?;
 
         let current_contract = current_contract(self.context, self.fp, self.memory)?;
 
-        let length = bytes::padded_len_usize(
-            length_unpadded
-                .try_into()
-                .map_err(|_| PanicReason::MemoryOverflow)?,
-        )
-        .map(|len| len as Word)
-        .unwrap_or(Word::MAX);
+        let length =
+            padded_len_word(length_unpadded).ok_or(PanicReason::MemoryOverflow)?;
 
         if length > self.contract_max_size {
             return Err(PanicReason::ContractMaxSize.into())
@@ -644,7 +637,7 @@ where
             owner,
             contract_bytes,
             region_start,
-            contract_offset,
+            contract_offset as usize,
             length,
         )?;
 
@@ -654,8 +647,8 @@ where
                 (*self.fp).saturating_add(CallFrame::code_size_offset() as Word);
             let old_code_size =
                 Word::from_be_bytes(self.memory.read_bytes(code_size_ptr)?);
-            let old_code_size = padded_len_word(old_code_size)
-                .expect("Code size cannot overflow with padding");
+            let old_code_size =
+                padded_len_word(old_code_size).ok_or(PanicReason::MemoryOverflow)?;
             let new_code_size = old_code_size
                 .checked_add(length as Word)
                 .ok_or(PanicReason::MemoryOverflow)?;
@@ -790,9 +783,8 @@ where
         S: InterpreterStorage,
     {
         let contract_id = ContractId::from(self.memory.read_bytes(contract_id_addr)?);
-        let offset: usize = contract_offset
-            .try_into()
-            .map_err(|_| PanicReason::MemoryOverflow)?;
+        let offset =
+            u32::try_from(contract_offset).map_err(|_| PanicReason::MemoryOverflow)?;
 
         self.memory.write(self.owner, dst_addr, length)?;
         self.input_contracts.check(&contract_id)?;
@@ -821,7 +813,7 @@ where
             self.owner,
             contract.as_ref().as_ref(),
             dst_addr,
-            offset,
+            offset as usize,
             length,
         )?;
 
@@ -901,7 +893,7 @@ impl<'vm, S> CodeRootCtx<'vm, S> {
 
         self.input_contracts.check(&contract_id)?;
 
-        let len = contract_size(self.storage, &contract_id)? as Word;
+        let len = contract_size(self.storage, &contract_id)?;
         let profiler = ProfileGas {
             pc: self.pc.as_ref(),
             is: self.is,
@@ -913,7 +905,7 @@ impl<'vm, S> CodeRootCtx<'vm, S> {
             self.ggas,
             profiler,
             self.gas_cost,
-            len,
+            len as u64,
         )?;
         let root = self
             .storage
@@ -955,7 +947,7 @@ impl<'vm, S> CodeSizeCtx<'vm, S> {
 
         self.input_contracts.check(&contract_id)?;
 
-        let len = contract_size(self.storage, &contract_id)? as Word;
+        let len = contract_size(self.storage, &contract_id)?;
         let profiler = ProfileGas {
             pc: self.pc.as_ref(),
             is: self.is,
@@ -967,9 +959,9 @@ impl<'vm, S> CodeSizeCtx<'vm, S> {
             self.ggas,
             profiler,
             self.gas_cost,
-            len,
+            len as u64,
         )?;
-        *result = len;
+        *result = len as u64;
 
         Ok(inc_pc(self.pc)?)
     }

diff --git a/fuel-vm/src/interpreter/contract.rs b/fuel-vm/src/interpreter/contract.rs
@@ -385,14 +385,15 @@ impl<'vm, S, Tx> TransferCtx<'vm, S, Tx> {
 pub(crate) fn contract_size<S>(
     storage: &S,
     contract: &ContractId,
-) -> IoResult<usize, S::Error>
+) -> IoResult<u32, S::Error>
 where
     S: StorageSize<ContractsRawCode> + ?Sized,
 {
-    Ok(storage
+    let size = storage
         .size_of_value(contract)
         .map_err(RuntimeError::Storage)?
-        .ok_or(PanicReason::ContractNotFound)?)
+        .ok_or(PanicReason::ContractNotFound)?;
+    Ok(u32::try_from(size).map_err(|_| PanicReason::MemoryOverflow)?)
 }
 
 pub(crate) fn balance<S>(

diff --git a/fuel-vm/src/interpreter/diff/tests.rs b/fuel-vm/src/interpreter/diff/tests.rs
@@ -107,7 +107,8 @@ fn reset_vm_state_frame() {
         Default::default(),
         Default::default(),
         Default::default(),
-    );
+    )
+    .unwrap();
     b.frames.push(frame);
     assert_ne!(a.frames, b.frames);
     let diff: Diff<InitialVmState> = a.diff(&b).into();

diff --git a/fuel-vm/src/interpreter/flow.rs b/fuel-vm/src/interpreter/flow.rs
@@ -477,9 +477,9 @@ where
         let asset_id =
             AssetId::new(self.memory.read_bytes(self.params.asset_id_pointer)?);
 
-        let code_size = contract_size(&self.storage, call.to())?;
+        let code_size = contract_size(&self.storage, call.to())? as usize;
         let code_size_padded =
-            padded_len_usize(code_size).expect("code_size cannot overflow with padding");
+            padded_len_usize(code_size).ok_or(PanicReason::MemoryOverflow)?;
 
         let total_size_in_stack = CallFrame::serialized_size()
             .checked_add(code_size_padded)
@@ -561,7 +561,8 @@ where
             code_size_padded,
             call.a(),
             call.b(),
-        );
+        )
+        .ok_or(PanicReason::MemoryOverflow)?;
         *frame.context_gas_mut() = *self.registers.system_registers.cgas;
         *frame.global_gas_mut() = *self.registers.system_registers.ggas;
 

diff --git a/fuel-vm/src/interpreter/flow/ret_tests.rs b/fuel-vm/src/interpreter/flow/ret_tests.rs
@@ -19,7 +19,8 @@ fn test_return() {
         0,
         0,
         0,
-    );
+    )
+    .unwrap();
     let mut frames = vec![frame];
     let mut registers = [0; VM_REGISTER_COUNT];
     registers[RegId::CGAS] = 99;

diff --git a/fuel-vm/src/interpreter/flow/tests.rs b/fuel-vm/src/interpreter/flow/tests.rs
@@ -119,7 +119,8 @@ impl Default for Output {
                 16,
                 0,
                 0,
-            )],
+            )
+            .unwrap()],
             receipts: vec![Receipt::call(
                 Default::default(),
                 Default::default(),
@@ -181,7 +182,7 @@ fn mem(set: &[(usize, Vec<u8>)]) -> MemoryInstance {
         script: Some(Default::default()),
         ..Default::default()
     } => using check_output({
-        let frame = CallFrame::new(ContractId::from([1u8; 32]), AssetId::from([2u8; 32]), make_reg(&[(HP, 1000), (SP, 200), (SSP, 200), (CGAS, 161), (GGAS, 191)]), 104, 4, 5);
+        let frame = CallFrame::new(ContractId::from([1u8; 32]), AssetId::from([2u8; 32]), make_reg(&[(HP, 1000), (SP, 200), (SSP, 200), (CGAS, 161), (GGAS, 191)]), 104, 4, 5).unwrap();
         let receipt = Receipt::call(ContractId::zeroed(), ContractId::from([1u8; 32]), 20, AssetId::from([2u8; 32]), 30, 4, 5, 800, 800);
         let mut script = Script::default();
         *script.receipts_root_mut() = crypto::ephemeral_merkle_root([receipt.to_bytes()].into_iter());
@@ -210,7 +211,7 @@ fn mem(set: &[(usize, Vec<u8>)]) -> MemoryInstance {
     } => using check_output(Ok(Output{
         reg: RegInput{hp: 1000, sp: 716, ssp: 716, fp: 100, pc: 700, is: 700, bal: 20, cgas: 0, ggas: 10 },
         receipts: vec![Receipt::call(Default::default(), Default::default(), 20, Default::default(), 0, 0, 0, 700, 700)].into(),
-        frames: vec![CallFrame::new(Default::default(), Default::default(), make_reg(&[(HP, 1000), (SP, 100), (SSP, 100), (CGAS, 10), (GGAS, 10)]), 16, 0, 0)],
+        frames: vec![CallFrame::new(Default::default(), Default::default(), make_reg(&[(HP, 1000), (SP, 100), (SSP, 100), (CGAS, 10), (GGAS, 10)]), 16, 0, 0).unwrap()],
         ..Default::default()
     })); "transfers with enough balance external"
 )]
@@ -229,7 +230,7 @@ fn mem(set: &[(usize, Vec<u8>)]) -> MemoryInstance {
     } => using check_output(Ok(Output{
         reg: RegInput{hp: 1000, sp: 716, ssp: 716, fp: 100, pc: 700, is: 700, bal: 20, cgas: 10, ggas: 79 },
         receipts: vec![Receipt::call(Default::default(), Default::default(), 20, Default::default(), 10, 0, 0, 700, 700)].into(),
-        frames: vec![CallFrame::new(Default::default(), Default::default(), make_reg(&[(HP, 1000), (SP, 100), (SSP, 100), (CGAS, 29), (GGAS, 79)]), 16, 0, 0)],
+        frames: vec![CallFrame::new(Default::default(), Default::default(), make_reg(&[(HP, 1000), (SP, 100), (SSP, 100), (CGAS, 29), (GGAS, 79)]), 16, 0, 0).unwrap()],
         ..Default::default()
     })); "forwards gas"
 )]
@@ -248,7 +249,7 @@ fn mem(set: &[(usize, Vec<u8>)]) -> MemoryInstance {
     } => using check_output(Ok(Output{
         reg: RegInput{hp: 1000, sp: 716, ssp: 716, fp: 100, pc: 700, is: 700, bal: 20, cgas: 39, ggas: 79 },
         receipts: vec![Receipt::call(Default::default(), Default::default(), 20, Default::default(), 39, 0, 0, 700, 700)].into(),
-        frames: vec![CallFrame::new(Default::default(), Default::default(), make_reg(&[(HP, 1000), (SP, 100), (SSP, 100), (CGAS, 0), (GGAS, 79)]), 16, 0, 0)],
+        frames: vec![CallFrame::new(Default::default(), Default::default(), make_reg(&[(HP, 1000), (SP, 100), (SSP, 100), (CGAS, 0), (GGAS, 79)]), 16, 0, 0).unwrap()],
         ..Default::default()
     })); "the receipt shows forwarded gas correctly when limited by available gas"
 )]
@@ -267,7 +268,7 @@ fn mem(set: &[(usize, Vec<u8>)]) -> MemoryInstance {
     } => using check_output(Ok(Output{
         reg: RegInput{hp: 1000, sp: 716, ssp: 716, fp: 100, pc: 700, is: 700, bal: 20, cgas: 0, ggas: 10 },
         receipts: vec![Receipt::call(Default::default(), Default::default(), 20, Default::default(), 0, 0, 0, 700, 700)].into(),
-        frames: vec![CallFrame::new(Default::default(), Default::default(), make_reg(&[(HP, 1000), (SP, 100), (SSP, 100), (CGAS, 10), (GGAS, 10)]), 16, 0, 0)],
+        frames: vec![CallFrame::new(Default::default(), Default::default(), make_reg(&[(HP, 1000), (SP, 100), (SSP, 100), (CGAS, 10), (GGAS, 10)]), 16, 0, 0).unwrap()],
         ..Default::default()
     })); "transfers with enough balance internal"
 )]

diff --git a/fuel-vm/src/lib.rs b/fuel-vm/src/lib.rs
@@ -32,6 +32,7 @@ mod convert;
 pub mod crypto;
 pub mod error;
 pub mod interpreter;
+#[cfg(feature = "test-helpers")]
 pub mod memory_client;
 pub mod pool;
 pub mod predicate;
@@ -146,7 +147,6 @@ pub mod prelude {
             MemoryInstance,
             MemoryRange,
         },
-        memory_client::MemoryClient,
         pool::VmMemoryPool,
         predicate::RuntimePredicate,
         state::{
@@ -157,7 +157,6 @@ pub mod prelude {
         },
         storage::{
             InterpreterStorage,
-            MemoryStorage,
             PredicateStorage,
         },
         transactor::Transactor,
@@ -169,12 +168,14 @@ pub mod prelude {
     };
 
     #[cfg(any(test, feature = "test-helpers"))]
-    pub use crate::util::test_helpers::TestBuilder;
-
-    #[cfg(any(test, feature = "test-helpers"))]
-    pub use crate::checked_transaction::{
-        builder::TransactionBuilderExt,
-        IntoChecked,
+    pub use crate::{
+        checked_transaction::{
+            builder::TransactionBuilderExt,
+            IntoChecked,
+        },
+        memory_client::MemoryClient,
+        storage::MemoryStorage,
+        util::test_helpers::TestBuilder,
     };
 
     #[cfg(all(