Remove unchecked arithmetic (#1831)

* Remove unchecked arithmetic

* Remove from erc20

* Remove from erc1155

* Remove from erc721

* Remove from incrementer

* Remove from payment-channel

* Remove from trait-erc20

* Remove from trait-incrementer

* Remove from multi-contract-caller

* Remove from multisig

* Remove from set-code-hash

* cargo fmt

* Spellcheck

crates/env/Cargo.toml

@@ -26,7 +26,6 @@ derive_more = { workspace = true, features = ["from", "display"] }
 num-traits = { workspace = true, features = ["i128"] }
 cfg-if = { workspace = true }
 paste = { workspace = true }
-arrayref = { workspace = true }
 static_assertions = { workspace = true }
 const_env = { workspace = true }
 

crates/env/src/call/execution_input.rs

@@ -184,7 +184,9 @@ where
 {
     #[inline]
     fn size_hint(&self) -> usize {
-        scale::Encode::size_hint(&self.head) + scale::Encode::size_hint(&self.rest)
+        scale::Encode::size_hint(&self.head)
+            .checked_add(scale::Encode::size_hint(&self.rest))
+            .unwrap()
     }
 
     #[inline]
@@ -204,7 +206,9 @@ where
 {
     #[inline]
     fn size_hint(&self) -> usize {
-        scale::Encode::size_hint(&self.selector) + scale::Encode::size_hint(&self.args)
+        scale::Encode::size_hint(&self.selector)
+            .checked_add(scale::Encode::size_hint(&self.args))
+            .unwrap()
     }
 
     #[inline]

crates/env/src/engine/mod.rs

@@ -31,6 +31,27 @@ use ink_primitives::{
     LangError,
 };
 
+/// Convert a slice into an array reference.
+///
+/// Creates an array reference of size `$len` pointing to `$offset` within `$arr`.
+///
+/// # Panics
+///
+/// - The selected range is out of bounds given the supplied slice
+/// - Integer overflow on `$offset + $len`
+macro_rules! array_mut_ref {
+    ($arr:expr, $offset:expr, $len:expr) => {{
+        {
+            fn as_array<T>(slice: &mut [T]) -> &mut [T; $len] {
+                slice.try_into().unwrap()
+            }
+            let offset: usize = $offset;
+            let slice = &mut $arr[offset..offset.checked_add($len).unwrap()];
+            as_array(slice)
+        }
+    }};
+}
+
 pub trait OnInstance: EnvBackend + TypedEnvBackend {
     fn on_instance<F, R>(f: F) -> R
     where

crates/env/src/engine/off_chain/impls.rs

@@ -64,7 +64,7 @@ impl CryptoHash for Blake2x128 {
             <Blake2x128 as HashOutput>::Type,
             OutputType
         );
-        let output: &mut OutputType = arrayref::array_mut_ref!(output, 0, 16);
+        let output: &mut OutputType = array_mut_ref!(output, 0, 16);
         Engine::hash_blake2_128(input, output);
     }
 }
@@ -76,7 +76,7 @@ impl CryptoHash for Blake2x256 {
             <Blake2x256 as HashOutput>::Type,
             OutputType
         );
-        let output: &mut OutputType = arrayref::array_mut_ref!(output, 0, 32);
+        let output: &mut OutputType = array_mut_ref!(output, 0, 32);
         Engine::hash_blake2_256(input, output);
     }
 }
@@ -88,7 +88,7 @@ impl CryptoHash for Sha2x256 {
             <Sha2x256 as HashOutput>::Type,
             OutputType
         );
-        let output: &mut OutputType = arrayref::array_mut_ref!(output, 0, 32);
+        let output: &mut OutputType = array_mut_ref!(output, 0, 32);
         Engine::hash_sha2_256(input, output);
     }
 }
@@ -100,7 +100,7 @@ impl CryptoHash for Keccak256 {
             <Keccak256 as HashOutput>::Type,
             OutputType
         );
-        let output: &mut OutputType = arrayref::array_mut_ref!(output, 0, 32);
+        let output: &mut OutputType = array_mut_ref!(output, 0, 32);
         Engine::hash_keccak_256(input, output);
     }
 }

crates/env/src/engine/on_chain/buffer.rs

@@ -82,15 +82,16 @@ impl<'a> EncodeScope<'a> {
 impl<'a> scale::Output for EncodeScope<'a> {
     fn write(&mut self, bytes: &[u8]) {
         debug_assert!(
-            self.len() + bytes.len() <= self.capacity(),
+            self.len().checked_add(bytes.len()).unwrap() <= self.capacity(),
             "encode scope buffer overflowed. capacity is {} but last write index is {}",
             self.capacity(),
-            self.len() + bytes.len(),
+            self.len().checked_add(bytes.len()).unwrap(),
         );
         let start = self.len;
         let len_bytes = bytes.len();
-        self.buffer[start..(start + len_bytes)].copy_from_slice(bytes);
-        self.len += len_bytes;
+        self.buffer[start..(start.checked_add(len_bytes)).unwrap()]
+            .copy_from_slice(bytes);
+        self.len = self.len.checked_add(len_bytes).unwrap();
     }
 
     fn push_byte(&mut self, byte: u8) {
@@ -101,7 +102,7 @@ impl<'a> scale::Output for EncodeScope<'a> {
             self.capacity(),
         );
         self.buffer[self.len] = byte;
-        self.len += 1;
+        self.len = self.len.checked_add(1).unwrap();
     }
 }
 
@@ -144,7 +145,7 @@ impl<'a> ScopedBuffer<'a> {
         self.buffer = rhs;
         debug_assert_eq!(lhs.len(), len);
         let len_after = self.buffer.len();
-        debug_assert_eq!(len_before - len_after, len);
+        debug_assert_eq!(len_before.checked_sub(len_after).unwrap(), len);
         lhs
     }
 
@@ -195,7 +196,7 @@ impl<'a> ScopedBuffer<'a> {
         let mut encode_scope = EncodeScope::from(&mut buffer[offset..]);
         scale::Encode::encode_to(&value, &mut encode_scope);
         let encode_len = encode_scope.len();
-        self.offset += encode_len;
+        self.offset = self.offset.checked_add(encode_len).unwrap();
         let _ = core::mem::replace(&mut self.buffer, buffer);
     }
 

crates/env/src/engine/on_chain/impls.rs

@@ -57,7 +57,7 @@ impl CryptoHash for Blake2x128 {
             <Blake2x128 as HashOutput>::Type,
             OutputType
         );
-        let output: &mut OutputType = arrayref::array_mut_ref!(output, 0, 16);
+        let output: &mut OutputType = array_mut_ref!(output, 0, 16);
         ext::hash_blake2_128(input, output);
     }
 }
@@ -69,7 +69,7 @@ impl CryptoHash for Blake2x256 {
             <Blake2x256 as HashOutput>::Type,
             OutputType
         );
-        let output: &mut OutputType = arrayref::array_mut_ref!(output, 0, 32);
+        let output: &mut OutputType = array_mut_ref!(output, 0, 32);
         ext::hash_blake2_256(input, output);
     }
 }
@@ -81,7 +81,7 @@ impl CryptoHash for Sha2x256 {
             <Sha2x256 as HashOutput>::Type,
             OutputType
         );
-        let output: &mut OutputType = arrayref::array_mut_ref!(output, 0, 32);
+        let output: &mut OutputType = array_mut_ref!(output, 0, 32);
         ext::hash_sha2_256(input, output);
     }
 }
@@ -93,7 +93,7 @@ impl CryptoHash for Keccak256 {
             <Keccak256 as HashOutput>::Type,
             OutputType
         );
-        let output: &mut OutputType = arrayref::array_mut_ref!(output, 0, 32);
+        let output: &mut OutputType = array_mut_ref!(output, 0, 32);
         ext::hash_keccak_256(input, output);
     }
 }

integration-tests/erc1155/lib.rs

@@ -267,7 +267,10 @@ mod erc1155 {
 
             // Given that TokenId is a `u128` the likelihood of this overflowing is pretty
             // slim.
-            self.token_id_nonce += 1;
+            #[allow(clippy::arithmetic_side_effects)]
+            {
+                self.token_id_nonce += 1;
+            }
             self.balances.insert((caller, self.token_id_nonce), &value);
 
             // Emit transfer event but with mint semantics
@@ -329,11 +332,15 @@ mod erc1155 {
                 .balances
                 .get((from, token_id))
                 .expect("Caller should have ensured that `from` holds `token_id`.");
-            sender_balance -= value;
+            // checks that sender_balance >= value were performed by caller
+            #[allow(clippy::arithmetic_side_effects)]
+            {
+                sender_balance -= value;
+            }
             self.balances.insert((from, token_id), &sender_balance);
 
             let mut recipient_balance = self.balances.get((to, token_id)).unwrap_or(0);
-            recipient_balance += value;
+            recipient_balance = recipient_balance.checked_add(value).unwrap();
             self.balances.insert((to, token_id), &recipient_balance);
 
             let caller = self.env().caller();

integration-tests/erc20/lib.rs

@@ -178,6 +178,8 @@ mod erc20 {
                 return Err(Error::InsufficientAllowance)
             }
             self.transfer_from_to(&from, &to, value)?;
+            // We checked that allowance >= value
+            #[allow(clippy::arithmetic_side_effects)]
             self.allowances
                 .insert((&from, &caller), &(allowance - value));
             Ok(())
@@ -201,10 +203,12 @@ mod erc20 {
             if from_balance < value {
                 return Err(Error::InsufficientBalance)
             }
-
+            // We checked that from_balance >= value
+            #[allow(clippy::arithmetic_side_effects)]
             self.balances.insert(from, &(from_balance - value));
             let to_balance = self.balance_of_impl(to);
-            self.balances.insert(to, &(to_balance + value));
+            self.balances
+                .insert(to, &(to_balance.checked_add(value).unwrap()));
             self.env().emit_event(Transfer {
                 from: Some(*from),
                 to: Some(*to),

integration-tests/erc721/lib.rs

@@ -227,7 +227,7 @@ mod erc721 {
 
             let count = owned_tokens_count
                 .get(caller)
-                .map(|c| c - 1)
+                .map(|c| c.checked_sub(1).unwrap())
                 .ok_or(Error::CannotFetchValue)?;
             owned_tokens_count.insert(caller, &count);
             token_owner.remove(id);
@@ -284,7 +284,7 @@ mod erc721 {
 
             let count = owned_tokens_count
                 .get(from)
-                .map(|c| c - 1)
+                .map(|c| c.checked_sub(1).unwrap())
                 .ok_or(Error::CannotFetchValue)?;
             owned_tokens_count.insert(from, &count);
             token_owner.remove(id);
@@ -308,7 +308,10 @@ mod erc721 {
                 return Err(Error::NotAllowed)
             };
 
-            let count = owned_tokens_count.get(to).map(|c| c + 1).unwrap_or(1);
+            let count = owned_tokens_count
+                .get(to)
+                .map(|c| c.checked_add(1).unwrap())
+                .unwrap_or(1);
 
             owned_tokens_count.insert(to, &count);
             token_owner.insert(id, to);

integration-tests/incrementer/lib.rs

@@ -25,7 +25,7 @@ mod incrementer {
 
         #[ink(message)]
         pub fn inc(&mut self, by: i32) {
-            self.value += by;
+            self.value = self.value.checked_add(by).unwrap();
         }
 
         #[ink(message)]

integration-tests/multi-contract-caller/accumulator/lib.rs

@@ -23,7 +23,7 @@ pub mod accumulator {
         /// Mutates the internal value.
         #[ink(message)]
         pub fn inc(&mut self, by: i32) {
-            self.value += by;
+            self.value = self.value.checked_add(by).unwrap();
         }
 
         /// Returns the current state.

integration-tests/multi-contract-caller/subber/lib.rs

@@ -26,7 +26,7 @@ mod subber {
         /// Decreases the `accumulator` value by some amount.
         #[ink(message)]
         pub fn dec(&mut self, by: i32) {
-            self.accumulator.inc(-by)
+            self.accumulator.inc(0i32.checked_sub(by).unwrap())
         }
     }
 }

integration-tests/multisig/lib.rs

@@ -379,7 +379,10 @@ mod multisig {
         pub fn add_owner(&mut self, new_owner: AccountId) {
             self.ensure_from_wallet();
             self.ensure_no_owner(&new_owner);
-            ensure_requirement_is_valid(self.owners.len() as u32 + 1, self.requirement);
+            ensure_requirement_is_valid(
+                (self.owners.len() as u32).checked_add(1).unwrap(),
+                self.requirement,
+            );
             self.is_owner.insert(new_owner, &());
             self.owners.push(new_owner);
             self.env().emit_event(OwnerAddition { owner: new_owner });
@@ -398,6 +401,8 @@ mod multisig {
         pub fn remove_owner(&mut self, owner: AccountId) {
             self.ensure_from_wallet();
             self.ensure_owner(&owner);
+            // If caller is an owner the len has to be > 0
+            #[allow(clippy::arithmetic_side_effects)]
             let len = self.owners.len() as u32 - 1;
             let requirement = u32::min(len, self.requirement);
             ensure_requirement_is_valid(len, requirement);
@@ -522,7 +527,10 @@ mod multisig {
                     "There is a entry in `self.confirmations`. Hence a count must exit.",
                 );
                 // Will not underflow as there is at least one confirmation
-                confirmation_count -= 1;
+                #[allow(clippy::arithmetic_side_effects)]
+                {
+                    confirmation_count -= 1;
+                }
                 self.confirmation_count
                     .insert(trans_id, &confirmation_count);
                 self.env().emit_event(Revocation {
@@ -618,14 +626,16 @@ mod multisig {
             let key = (transaction, confirmer);
             let new_confirmation = !self.confirmations.contains(key);
             if new_confirmation {
-                count += 1;
+                count = count.checked_add(1).unwrap();
                 self.confirmations.insert(key, &());
                 self.confirmation_count.insert(transaction, &count);
             }
             let status = {
                 if count >= self.requirement {
                     ConfirmationStatus::Confirmed
                 } else {
+                    // We checked that count < self.requirement
+                    #[allow(clippy::arithmetic_side_effects)]
                     ConfirmationStatus::ConfirmationsNeeded(self.requirement - count)
                 }
             };
@@ -677,7 +687,7 @@ mod multisig {
                 if self.confirmations.contains(key) {
                     self.confirmations.remove(key);
                     let mut count = self.confirmation_count.get(trans_id).unwrap_or(0);
-                    count -= 1;
+                    count = count.saturating_sub(1);
                     self.confirmation_count.insert(trans_id, &count);
                 }
             }

integration-tests/payment-channel/lib.rs

@@ -138,6 +138,8 @@ mod payment_channel {
                 return Err(Error::InvalidSignature)
             }
 
+            // We checked that amount >= self.withdrawn
+            #[allow(clippy::arithmetic_side_effects)]
             self.env()
                 .transfer(self.recipient, amount - self.withdrawn)
                 .map_err(|_| Error::TransferFailed)?;
@@ -157,7 +159,7 @@ mod payment_channel {
             }
 
             let now = self.env().block_timestamp();
-            let expiration = now + self.close_duration;
+            let expiration = now.checked_add(self.close_duration).unwrap();
 
             self.env().emit_event(SenderCloseStarted {
                 expiration,
@@ -207,8 +209,10 @@ mod payment_channel {
                 return Err(Error::AmountIsLessThanWithdrawn)
             }
 
+            // We checked that amount >= self.withdrawn
+            #[allow(clippy::arithmetic_side_effects)]
             let amount_to_withdraw = amount - self.withdrawn;
-            self.withdrawn += amount_to_withdraw;
+            self.withdrawn.checked_add(amount_to_withdraw).unwrap();
 
             self.env()
                 .transfer(self.recipient, amount_to_withdraw)

integration-tests/set-code-hash/lib.rs

@@ -36,7 +36,7 @@ pub mod incrementer {
         /// Increments the counter value which is stored in the contract's storage.
         #[ink(message)]
         pub fn inc(&mut self) {
-            self.count += 1;
+            self.count = self.count.checked_add(1).unwrap();
             ink::env::debug_println!(
                 "The new count is {}, it was modified using the original contract code.",
                 self.count