Skip to content

Commit

Permalink
refactor: remove load_word in favor of FromBytes (#988)
Browse files Browse the repository at this point in the history 
* load word to bytes

* unwrap and nits
  • Loading branch information
@lordshashank
lordshashank authored Sep 30, 2024
1 parent 7c289ed commit 4ce7e2d
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 110 deletions.
8 changes: 5 additions & 3 deletions crates/evm/src/instructions/environmental_information.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ use crate::model::vm::{VM, VMTrait};
use crate::model::{AddressTrait};
use crate::stack::StackTrait;
use crate::state::StateTrait;
use utils::helpers::{bytes_32_words_size, load_word};
use utils::helpers::bytes_32_words_size;
use utils::set::SetTrait;
use utils::traits::bytes::FromBytes;
use utils::traits::{EthAddressIntoU256};


Expand Down Expand Up @@ -85,8 +86,9 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {
let bytes_len = core::cmp::min(32, calldata_len - offset);
let sliced = calldata.slice(offset, bytes_len);

// Fill data to load with bytes in calldata
let mut data_to_load: u256 = load_word(bytes_len, sliced);
let mut data_to_load: u256 = sliced
.from_be_bytes_partial()
.expect('Failed to parse calldata');

// Fill the rest of the data to load with zeros
// TODO: optimize once we have dw-based exponentiation
Expand Down
26 changes: 17 additions & 9 deletions crates/evm/src/instructions/system_operations.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,8 @@ mod tests {
use snforge_std::{test_address, start_mock_call};
use utils::constants::EMPTY_KECCAK;
use utils::helpers::compute_starknet_address;
use utils::helpers::load_word;
use utils::traits::bytes::U8SpanExTrait;
use utils::traits::bytes::{U8SpanExTrait, FromBytes};

use utils::traits::{EthAddressIntoU256};


Expand All @@ -405,8 +405,11 @@ mod tests {
vm.stack.push(0).expect('push failed');
assert(vm.exec_return().is_ok(), 'Exec return failed');

// Then
assert(1000 == load_word(32, vm.return_data()), 'Wrong return_data');
let return_data = vm.return_data();
let parsed_return_data: u256 = return_data
.from_be_bytes()
.expect('Failed to parse return data');
assert(1000 == parsed_return_data, 'Wrong return_data');
assert(!vm.is_running(), 'vm should be stopped');
assert_eq!(vm.error, false);
}
Expand All @@ -424,8 +427,11 @@ mod tests {
vm.stack.push(0).expect('push failed');
assert(vm.exec_revert().is_ok(), 'Exec revert failed');

// Then
assert(1000 == load_word(32, vm.return_data()), 'Wrong return_data');
let return_data = vm.return_data();
let parsed_return_data: u256 = return_data
.from_be_bytes()
.expect('Failed to parse return data');
assert(1000 == parsed_return_data, 'Wrong return_data');
assert(!vm.is_running(), 'vm should be stopped');
assert_eq!(vm.error, true);
}
Expand All @@ -442,9 +448,11 @@ mod tests {
vm.stack.push(32).expect('push failed');
vm.stack.push(1).expect('push failed');
assert(vm.exec_return().is_ok(), 'Exec return failed');

// Then
assert(256 == load_word(32, vm.return_data()), 'Wrong return_data');
let return_data = vm.return_data();
let parsed_return_data: u256 = return_data
.from_be_bytes_partial()
.expect('Failed to parse return data');
assert(256 == parsed_return_data, 'Wrong return_data');
assert(!vm.is_running(), 'vm should be stopped');
assert_eq!(vm.error, false);
}
Expand Down
20 changes: 17 additions & 3 deletions crates/evm/src/memory.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use utils::constants::{
POW_2_72, POW_2_80, POW_2_88, POW_2_96, POW_2_104, POW_2_112, POW_2_120, POW_256_16
};
use utils::traits::array::ArrayExtTrait;
use utils::traits::bytes::FromBytes;
use utils::{helpers, math::Bitshift};

#[derive(Destruct, Default)]
Expand Down Expand Up @@ -307,7 +308,10 @@ pub(crate) impl InternalMemoryMethods of InternalMemoryTrait {
let nonzero_mask_f: NonZero<u256> = mask_f.try_into().unwrap();
let (word_high, word_low) = DivRem::div_rem(word.into(), nonzero_mask_i);
let (_, word_low_l) = DivRem::div_rem(word_low, nonzero_mask_f);
let bytes_as_word = helpers::load_word(elements.len(), elements);
let bytes_as_word: u128 = elements
.slice(0, elements.len())
.from_be_bytes_partial()
.expect('Failed to parse word_low');
let new_w: u128 = (word_high * mask_i + bytes_as_word.into() * mask_f + word_low_l)
.try_into()
.unwrap();
Expand Down Expand Up @@ -545,7 +549,14 @@ pub(crate) impl InternalMemoryMethods of InternalMemoryTrait {
) {
let word = self.items.get(chunk_index.into());
let word_high = (word.into() / start_mask);
let word_low = helpers::load_word(16 - start_offset_in_chunk, elements);

let bytes_to_read = 16 - start_offset_in_chunk;

let word_low: u128 = elements
.slice(0, bytes_to_read)
.from_be_bytes_partial()
.expect('Failed to parse word_low');

let new_word: u128 = (word_high * start_mask + word_low.into()).try_into().unwrap();
self.items.insert(chunk_index.into(), new_word);
}
Expand Down Expand Up @@ -579,7 +590,10 @@ pub(crate) impl InternalMemoryMethods of InternalMemoryTrait {
let word = self.items.get(chunk_index.into());
let word_low = (word.into() % end_mask);

let low_bytes = helpers::load_word(end_offset_in_chunk, elements);
let low_bytes: u128 = elements
.slice(0, end_offset_in_chunk)
.from_be_bytes_partial()
.expect('Failed to parse low_bytes');
let new_word: u128 = (low_bytes.into() * end_mask + word_low).try_into().unwrap();
self.items.insert(chunk_index.into(), new_word);
}
Expand Down
15 changes: 5 additions & 10 deletions crates/evm/src/precompiles/ec_operations/ec_add.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ use crate::precompiles::ec_operations::{
eq_mod_p, eq_neg_mod_p, is_on_curve, double_ec_point_unchecked, BN254_PRIME_LIMBS, BN254_PRIME
};
use garaga::core::circuit::AddInputResultTrait2;
use utils::helpers::{load_word};
use utils::traits::bytes::{ToBytes, U8SpanExTrait};
use utils::traits::bytes::{ToBytes, U8SpanExTrait, FromBytes};


const BASE_COST: u64 = 150;
Expand All @@ -31,17 +30,13 @@ pub impl EcAdd of Precompile {
// Pad the input to 128 bytes to avoid out-of-bounds accesses
let mut input = input.pad_right_with_zeroes(128);

let x1_bytes = *(input.multi_pop_front::<32>().unwrap());
let x1: u256 = load_word(U256_BYTES_LEN, x1_bytes.unbox().span());
let x1: u256 = input.slice(0, 32).from_be_bytes().unwrap();

let y1_bytes = *(input.multi_pop_front::<32>().unwrap());
let y1: u256 = load_word(U256_BYTES_LEN, y1_bytes.unbox().span());
let y1: u256 = input.slice(32, 32).from_be_bytes().unwrap();

let x2_bytes = *(input.multi_pop_front::<32>().unwrap());
let x2: u256 = load_word(U256_BYTES_LEN, x2_bytes.unbox().span());
let x2: u256 = input.slice(64, 32).from_be_bytes().unwrap();

let y2_bytes = *(input.multi_pop_front::<32>().unwrap());
let y2: u256 = load_word(U256_BYTES_LEN, y2_bytes.unbox().span());
let y2: u256 = input.slice(96, 32).from_be_bytes().unwrap();

let (x, y) = match ec_add(x1, y1, x2, y2) {
Option::Some((x, y)) => { (x, y) },
Expand Down
12 changes: 4 additions & 8 deletions crates/evm/src/precompiles/ec_operations/ec_mul.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ use crate::errors::EVMError;
use crate::precompiles::Precompile;
use crate::precompiles::ec_operations::ec_add::ec_safe_add;
use crate::precompiles::ec_operations::{is_on_curve, double_ec_point_unchecked, BN254_PRIME};
use utils::helpers::{load_word};
use utils::traits::bytes::{ToBytes, U8SpanExTrait};
use utils::traits::bytes::{ToBytes, U8SpanExTrait, FromBytes};

const BASE_COST: u64 = 6000;
const U256_BYTES_LEN: usize = 32;
Expand All @@ -23,14 +22,11 @@ pub impl EcMul of Precompile {
// Pad the input to 128 bytes to avoid out-of-bounds accesses
let mut input = input.pad_right_with_zeroes(96);

let x1_bytes = *(input.multi_pop_front::<32>().unwrap());
let x1: u256 = load_word(U256_BYTES_LEN, x1_bytes.unbox().span());
let x1: u256 = input.slice(0, 32).from_be_bytes().unwrap();

let y1_bytes = *(input.multi_pop_front::<32>().unwrap());
let y1: u256 = load_word(U256_BYTES_LEN, y1_bytes.unbox().span());
let y1: u256 = input.slice(32, 32).from_be_bytes().unwrap();

let s_bytes = *(input.multi_pop_front::<32>().unwrap());
let s: u256 = load_word(U256_BYTES_LEN, s_bytes.unbox().span());
let s: u256 = input.slice(64, 32).from_be_bytes().unwrap();

let (x, y) = match ec_mul(x1, y1, s) {
Option::Some((x, y)) => { (x, y) },
Expand Down
77 changes: 0 additions & 77 deletions crates/utils/src/helpers.cairo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,34 +112,6 @@ pub fn split_word_128(value: u256, ref dst: Array<u8>) {
split_word(value, 16, ref dst)
}


/// Loads a sequence of bytes into a single u256 in big-endian order.
///
/// # Arguments
/// * `len` - The number of bytes to load.
/// * `words` - The span of bytes to load.
///
/// # Returns
/// A `u256` value representing the loaded bytes in big-endian order.
pub fn load_word(mut len: usize, words: Span<u8>) -> u256 {
if len == 0 {
return 0;
}

let mut current: u256 = 0;
let mut counter = 0;

for _ in 0
..len {
let loaded: u8 = *words[counter];
let tmp = current * 256;
current = tmp + loaded.into();
counter += 1;
};

current
}

/// Converts a u256 to a bytes array represented by an array of u8 values in big-endian order.
///
/// # Arguments
Expand Down Expand Up @@ -226,55 +198,6 @@ mod tests {
assert(1 == *bytes_array[30], 'wrong conversion');
}

#[test]
fn test_load_word() {
// No bytes to load
let res0 = helpers::load_word(0, ArrayTrait::new().span());
assert(0 == res0, 'res0: wrong load');

// Single bytes value
let mut arr1 = ArrayTrait::new();
arr1.append(0x01);
let res1 = helpers::load_word(1, arr1.span());
assert(1 == res1, 'res1: wrong load');

let mut arr2 = ArrayTrait::new();
arr2.append(0xff);
let res2 = helpers::load_word(1, arr2.span());
assert(255 == res2, 'res2: wrong load');

// Two byte values
let mut arr3 = ArrayTrait::new();
arr3.append(0x01);
arr3.append(0x00);
let res3 = helpers::load_word(2, arr3.span());
assert(256 == res3, 'res3: wrong load');

let mut arr4 = ArrayTrait::new();
arr4.append(0xff);
arr4.append(0xff);
let res4 = helpers::load_word(2, arr4.span());
assert(65535 == res4, 'res4: wrong load');

// Four byte values
let mut arr5 = ArrayTrait::new();
arr5.append(0xff);
arr5.append(0xff);
arr5.append(0xff);
arr5.append(0xff);
let res5 = helpers::load_word(4, arr5.span());
assert(4294967295 == res5, 'res5: wrong load');

// 16 bytes values
let mut arr6 = ArrayTrait::new();
for _ in 0..16_u8 {
arr6.append(0xff);
};
let res6 = helpers::load_word(16, arr6.span());
assert(340282366920938463463374607431768211455 == res6, 'res6: wrong load');
}


#[test]
fn test_split_word_le() {
// Test with 0 value and 0 len
Expand Down

0 comments on commit 4ce7e2d

Please sign in to comment.