Some refactoring and removed unwraps for byte arrays. (#6853)

corelib/src/byte_array.cairo

@@ -45,8 +45,8 @@
 use crate::array::{ArrayTrait, SpanTrait};
 #[allow(unused_imports)]
 use crate::bytes_31::{
-    BYTES_IN_BYTES31, Bytes31Trait, one_shift_left_bytes_felt252, one_shift_left_bytes_u128,
-    POW_2_128, POW_2_8, U128IntoBytes31, U8IntoBytes31,
+    BYTES_IN_BYTES31, Bytes31Trait, POW_2_128, POW_2_8, U128IntoBytes31, U8IntoBytes31,
+    one_shift_left_bytes_felt252, one_shift_left_bytes_u128, split_u128, u8_at_u256,
 };
 use crate::clone::Clone;
 use crate::cmp::min;
@@ -277,29 +277,21 @@ pub impl ByteArrayImpl of ByteArrayTrait {
     /// assert!(byte == 98);
     /// ```
     fn at(self: @ByteArray, index: usize) -> Option<u8> {
-        let (word_index, index_in_word) = DivRem::div_rem(
-            index, BYTES_IN_BYTES31.try_into().unwrap(),
-        );
-
-        let data_len = self.data.len();
-        if word_index == data_len {
+        let (word_index, index_in_word) = DivRem::div_rem(index, 31);
+        if word_index == self.data.len() {
             // Index is in pending word.
             if index_in_word >= *self.pending_word_len {
                 return Option::None;
             }
             // index_in_word is from MSB, we need index from LSB.
-            let index_from_lsb = *self.pending_word_len - 1 - index_in_word;
-            let pending_bytes31: bytes31 = (*self.pending_word).try_into().unwrap();
-            return Option::Some(pending_bytes31.at(index_from_lsb));
-        }
-
-        if word_index > data_len {
-            return Option::None;
+            return Option::Some(
+                u8_at_u256((*self.pending_word).into(), *self.pending_word_len - 1 - index_in_word),
+            );
         }
 
+        let data_word: bytes31 = *self.data.get(word_index)?.deref();
         // index_in_word is from MSB, we need index from LSB.
-        let index_from_lsb = BYTES_IN_BYTES31 - 1 - index_in_word;
-        Option::Some(self.data.at(word_index).at(index_from_lsb))
+        Option::Some(data_word.at(BYTES_IN_BYTES31 - 1 - index_in_word))
     }
 
     /// Returns a `ByteArray` with the reverse order of `self`.
@@ -350,9 +342,7 @@ pub impl ByteArrayImpl of ByteArrayTrait {
             if index == low_part_limit {
                 break;
             }
-            let curr_byte_as_u128 = (low / one_shift_left_bytes_u128(index)) % POW_2_8;
-
-            self.append_byte(curr_byte_as_u128.try_into().unwrap());
+            self.append_byte(core::bytes_31::get_lsb(split_u128(low, index).high));
             index += 1;
         };
         if low_part_limit == BYTES_IN_U128 {
@@ -362,10 +352,10 @@ pub impl ByteArrayImpl of ByteArrayTrait {
                 if index_in_high_part == high_part_len {
                     break;
                 }
-                let curr_byte_as_u128 = (high
-                    / one_shift_left_bytes_u128(index_in_high_part)) % POW_2_8;
-
-                self.append_byte(curr_byte_as_u128.try_into().unwrap());
+                self
+                    .append_byte(
+                        core::bytes_31::get_lsb(split_u128(high, index_in_high_part).high),
+                    );
                 index_in_high_part += 1;
             }
         }
@@ -402,13 +392,11 @@ pub impl ByteArrayImpl of ByteArrayTrait {
     fn append_split_index_lt_16(ref self: ByteArray, word: felt252, split_index: usize) {
         let u256 { low, high } = word.into();
 
-        let (low_quotient, low_remainder) = u128_safe_divmod(
-            low, one_shift_left_bytes_u128(split_index).try_into().unwrap(),
-        );
+        let low_result = split_u128(low, split_index);
         let left = high.into() * one_shift_left_bytes_u128(BYTES_IN_U128 - split_index).into()
-            + low_quotient.into();
+            + low_result.high.into();
 
-        self.append_split(left, low_remainder.into());
+        self.append_split(left, low_result.low.into());
     }
 
     /// Appends a single word to the end of `self`, given that the index of splitting `word` is
@@ -437,12 +425,10 @@ pub impl ByteArrayImpl of ByteArrayTrait {
     fn append_split_index_gt_16(ref self: ByteArray, word: felt252, split_index: usize) {
         let u256 { low, high } = word.into();
 
-        let (high_quotient, high_remainder) = u128_safe_divmod(
-            high, one_shift_left_bytes_u128(split_index - BYTES_IN_U128).try_into().unwrap(),
-        );
-        let right = high_remainder.into() * POW_2_128 + low.into();
+        let high_result = split_u128(high, split_index - BYTES_IN_U128);
+        let right = high_result.low.into() * POW_2_128 + low.into();
 
-        self.append_split(high_quotient.into(), right);
+        self.append_split(high_result.high.into(), right);
     }
 
     /// A helper function to append a remainder to `self`, by:

corelib/src/bytes_31.cairo

@@ -16,6 +16,7 @@
 
 #[allow(unused_imports)]
 use crate::integer::{u128_safe_divmod, u128_to_felt252};
+#[allow(unused_imports)]
 use crate::option::OptionTrait;
 use crate::RangeCheck;
 use crate::traits::{Into, TryInto};
@@ -48,13 +49,7 @@ pub impl Bytes31Impl of Bytes31Trait {
     /// assert!(bytes.at(0) == 1);
     /// ```
     fn at(self: @bytes31, index: usize) -> u8 {
-        let u256 { low, high } = (*self).into();
-        let res_u128 = if index < BYTES_IN_U128 {
-            (low / one_shift_left_bytes_u128(index)) % POW_2_8
-        } else {
-            (high / one_shift_left_bytes_u128(index - BYTES_IN_U128)) % POW_2_8
-        };
-        res_u128.try_into().unwrap()
+        u8_at_u256((*self).into(), index)
     }
 }
 
@@ -147,28 +142,23 @@ pub(crate) fn split_bytes31(word: felt252, len: usize, index: usize) -> (felt252
     }
 
     if len <= BYTES_IN_U128 {
-        let (quotient, remainder) = u128_safe_divmod(
-            low, one_shift_left_bytes_u128(index).try_into().unwrap(),
-        );
-        return (remainder.into(), quotient.into());
+        let result = split_u128(low, index);
+        return (result.low.into(), result.high.into());
     }
 
     // len > BYTES_IN_U128
     if index < BYTES_IN_U128 {
-        let (low_quotient, low_remainder) = u128_safe_divmod(
-            low, one_shift_left_bytes_u128(index).try_into().unwrap(),
-        );
+        let low_result = split_u128(low, index);
         let right = high.into() * one_shift_left_bytes_u128(BYTES_IN_U128 - index).into()
-            + low_quotient.into();
-        return (low_remainder.into(), right);
+            + low_result.high.into();
+        return (low_result.low.into(), right);
     }
 
     // len > BYTES_IN_U128 && index > BYTES_IN_U128
-    let (high_quotient, high_remainder) = u128_safe_divmod(
-        high, one_shift_left_bytes_u128(index - BYTES_IN_U128).try_into().unwrap(),
-    );
-    let left = high_remainder.into() * POW_2_128 + low.into();
-    return (left, high_quotient.into());
+
+    let high_result = split_u128(high, index - BYTES_IN_U128);
+    let left = high_result.low.into() * POW_2_128 + low.into();
+    return (left, high_result.high.into());
 }
 
 
@@ -189,6 +179,29 @@ pub(crate) fn one_shift_left_bytes_felt252(n_bytes: usize) -> felt252 {
 ///
 /// Panics if `n_bytes >= BYTES_IN_U128`.
 pub(crate) fn one_shift_left_bytes_u128(n_bytes: usize) -> u128 {
+    one_shift_left_bytes_u128_nz(n_bytes).into()
+}
+
+/// Splits a u128 into two words as a `u256` - the `low` is the first `n_bytes` the `high` is the
+/// rest.
+pub(crate) fn split_u128(value: u128, n_bytes: usize) -> u256 {
+    let (high, low) = DivRem::div_rem(value, one_shift_left_bytes_u128_nz(n_bytes));
+    u256 { low, high }
+}
+
+/// Returns the `u8` at `index` if you look at `value` as an array of 32 `u8`s.
+pub(crate) fn u8_at_u256(value: u256, index: usize) -> u8 {
+    get_lsb(
+        if index < BYTES_IN_U128 {
+            split_u128(value.low, index).high
+        } else {
+            split_u128(value.high, index - BYTES_IN_U128).high
+        },
+    )
+}
+
+/// Same as `one_shift_left_bytes_u128` but returns `NonZero` value.
+fn one_shift_left_bytes_u128_nz(n_bytes: usize) -> NonZero<u128> {
     match n_bytes {
         0 => 0x1,
         1 => 0x100,
@@ -217,3 +230,20 @@ impl Bytes31PartialEq of PartialEq<bytes31> {
         lhs_as_felt252 == rhs_as_felt252
     }
 }
+
+mod helpers {
+    use core::internal::bounded_int::{DivRemHelper, BoundedInt, div_rem};
+
+    impl DivRemU128By256 of DivRemHelper<u128, BoundedInt<256, 256>> {
+        type DivT = BoundedInt<0, 0xffffffffffffffffffffffffffffff>;
+        type RemT = BoundedInt<0, 0xff>;
+    }
+
+    /// Returns the least significant byte of the given u128.
+    pub fn get_lsb(value: u128) -> u8 {
+        let (_, res) = div_rem::<_, BoundedInt<256, 256>>(value, 256);
+        core::integer::upcast(res)
+    }
+}
+
+pub(crate) use helpers::get_lsb;

crates/cairo-lang-starknet/cairo_level_tests/abi_dispatchers_tests.cairo

@@ -117,7 +117,7 @@ fn test_validate_gas_cost() {
     assert!(
         call_building_gas_usage == 3250
             && serialization_gas_usage == 42670
-            && entry_point_gas_usage == 143500,
+            && entry_point_gas_usage == 143000,
         "Unexpected gas_usage:
      call_building: `{call_building_gas_usage}`.
      serialization: `{serialization_gas_usage}`.

crates/cairo-lang-starknet/cairo_level_tests/contracts/libfuncs_coverage.cairo

@@ -119,6 +119,7 @@ enum Felt252TryIntoLibfuncs {
     I32: felt252,
     I64: felt252,
     I128: felt252,
+    Bytes31: felt252,
     ContractAddress: felt252,
     ClassHash: felt252,
     StorageAddress: felt252,
@@ -341,6 +342,7 @@ fn felt252_try_into_libfuncs(libfuncs: Felt252TryIntoLibfuncs) {
         Felt252TryIntoLibfuncs::I32(v) => use_and_panic::<Option<i32>>(v.try_into()),
         Felt252TryIntoLibfuncs::I64(v) => use_and_panic::<Option<i64>>(v.try_into()),
         Felt252TryIntoLibfuncs::I128(v) => use_and_panic::<Option<i128>>(v.try_into()),
+        Felt252TryIntoLibfuncs::Bytes31(v) => use_and_panic::<Option<bytes31>>(v.try_into()),
         Felt252TryIntoLibfuncs::ContractAddress(v) => use_and_panic::<
             Option<starknet::ContractAddress>,
         >(v.try_into()),