Skip to content

Commit

Permalink
refactor: replace while loops with for (#987)
Browse files Browse the repository at this point in the history
* First batch of replacements

* Second batch of replacements

* Clean commented code

* scarb fmt

* Apply code review recommendations
  • Loading branch information
fabrobles92 authored Sep 30, 2024
1 parent 88479f3 commit c645fee
Show file tree
Hide file tree
Showing 24 changed files with 234 additions and 309 deletions.
20 changes: 9 additions & 11 deletions crates/contracts/src/storage.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub struct StorageBytecode {

const BYTES_PER_FELT: NonZero<u32> = 31;

/// An implamentation of the `Store` trait for our specific `StorageBytecode` type.
/// An implementation of the `Store` trait for our specific `StorageBytecode` type.
/// The packing-unpacking is done inside the `read` and `write` methods, thus transparent to the
/// user.
/// The bytecode is stored sequentially, starting from storage address 0, for compatibility purposes
Expand All @@ -38,13 +38,13 @@ impl StoreBytecode of Store<StorageBytecode> {
// afterwards.
let base: felt252 = 0;
let mut packed_bytecode = array![];
let mut i = 0;
while i != (chunks_count + 1) {
let storage_address: StorageAddress = (base + i.into()).try_into().unwrap();
let chunk = storage_read_syscall(address_domain, storage_address).unwrap();
packed_bytecode.append(chunk);
i += 1;
};
for i in 0
..chunks_count
+ 1 {
let storage_address: StorageAddress = (base + i.into()).try_into().unwrap();
let chunk = storage_read_syscall(address_domain, storage_address).unwrap();
packed_bytecode.append(chunk);
};
let bytecode = load_packed_bytes(packed_bytecode.span(), bytecode_len);
SyscallResult::Ok(StorageBytecode { bytecode: bytecode.span() })
}
Expand Down Expand Up @@ -131,10 +131,8 @@ mod tests {
fn test_store_bytecode_multiple_chunks() {
let mut state = account_contract_state();
let mut bytecode_array = array![];
let mut i = 0;
while i != 100 {
for i in 0..100_u8 {
bytecode_array.append(i);
i += 1;
};
let bytecode = bytecode_array.span();
// Write the bytecode to the storage
Expand Down
4 changes: 2 additions & 2 deletions crates/evm/src/backend/starknet_backend.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ pub fn fetch_balance(self: @Address) -> u256 {
/// `Ok(())` if the commit was successful, otherwise an `EVMError`.
fn commit_accounts(ref state: State) -> Result<(), EVMError> {
let mut account_keys = state.accounts.keyset.to_span();
while let Option::Some(evm_address) = account_keys.pop_front() {
for evm_address in account_keys {
let account = state.accounts.changes.get(*evm_address).deref();
commit_account(@account, ref state);
};
Expand Down Expand Up @@ -231,7 +231,7 @@ fn emit_events(ref self: State) -> Result<(), EVMError> {
/// commit_storage MUST be called after commit_accounts.
fn commit_storage(ref self: State) -> Result<(), EVMError> {
let mut storage_keys = self.accounts_storage.keyset.to_span();
while let Option::Some(state_key) = storage_keys.pop_front() {
for state_key in storage_keys {
let (evm_address, key, value) = self.accounts_storage.changes.get(*state_key).deref();
let mut account = self.get_account(evm_address);
// @dev: EIP-6780 - If selfdestruct on an account created, dont commit data
Expand Down
11 changes: 3 additions & 8 deletions crates/evm/src/instructions/duplication_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -123,25 +123,20 @@ mod tests {

// ensures all values start from index `from` upto index `to` of stack are `0x0`
fn ensures_zeros(ref stack: Stack, from: u32, to: u32) {
let mut idx: u32 = from;

if to > from {
return;
}

while idx != to {
for idx in from..to {
assert(stack.peek_at(idx).unwrap() == 0x00, 'should be zero');
idx += 1;
};
}

// push `n` number of `0x0` to the stack
fn push_zeros(ref stack: Stack, n: u8) {
let mut i = 0;
while i != n {
for _ in 0..n {
stack.push(0x0).unwrap();
i += 1;
}
};
}

#[test]
Expand Down
55 changes: 26 additions & 29 deletions crates/evm/src/instructions/environmental_information.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,8 @@ pub impl EnvironmentInformationImpl of EnvironmentInformationTrait {

// Fill the rest of the data to load with zeros
// TODO: optimize once we have dw-based exponentiation
let mut i = 32 - bytes_len;
while i != 0 {
for _ in 0..32 - bytes_len {
data_to_load *= 256;
i -= 1;
};
self.stack.push(data_to_load)
}
Expand Down Expand Up @@ -652,21 +650,22 @@ mod tests {
// Memory initialization with a value to verify that if the offset + size is out of the
// bound bytes, 0's have been copied.
// Otherwise, the memory value would be 0, and we wouldn't be able to check it.
let mut i = 0;
while i != (size / 32) + 1 {
vm
.memory
.store(
0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF,
dest_offset + (i * 32)
);

let initial: u256 = vm.memory.load_internal(dest_offset + (i * 32)).into();

assert_eq!(initial, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF);

i += 1;
};
for i in 0
..(size / 32)
+ 1 {
vm
.memory
.store(
0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF,
dest_offset + (i * 32)
);

let initial: u256 = vm.memory.load_internal(dest_offset + (i * 32)).into();

assert_eq!(
initial, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
);
};

// When
vm.exec_calldatacopy().expect('exec_calldatacopy failed');
Expand Down Expand Up @@ -775,17 +774,15 @@ mod tests {
let result: u256 = vm.memory.load_internal(dest_offset).into();
let mut results: Array<u8> = u256_to_bytes_array(result);

let mut i = 0;
while i != size {
// For out of bound bytes, 0s will be copied.
if (i + offset >= bytecode.len()) {
assert_eq!(*results[i], 0);
} else {
assert_eq!(*results[i], *bytecode[i + offset]);
}

i += 1;
};
for i in 0
..size {
// For out of bound bytes, 0s will be copied.
if (i + offset >= bytecode.len()) {
assert_eq!(*results[i], 0);
} else {
assert_eq!(*results[i], *bytecode[i + offset]);
}
};
}

// *************************************************************************
Expand Down
3 changes: 1 addition & 2 deletions crates/evm/src/instructions/push_operations.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,8 @@ mod tests {
fn get_n_0xFF(mut n: u8) -> Span<u8> {
let mut array: Array<u8> = ArrayTrait::new();
array.append(0x00);
while n != 0 {
for _ in 0..n {
array.append(0xFF);
n -= 1;
};
array.span()
}
Expand Down
22 changes: 11 additions & 11 deletions crates/evm/src/instructions/sha3.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -102,17 +102,17 @@ fn compute_memory_words_amount(size: u32, offset: u32, mem_len: u32) -> (u32, u3
fn fill_array_with_memory_words(
ref self: VM, ref to_hash: Array<u64>, mut offset: u32, mut amount: u32
) -> u32 {
while amount != 0 {
let loaded = self.memory.load(offset);
let ((high_h, low_h), (high_l, low_l)) = loaded.split_into_u64_le();
to_hash.append(low_h);
to_hash.append(high_h);
to_hash.append(low_l);
to_hash.append(high_l);

offset += 32;
amount -= 1;
};
for _ in 0
..amount {
let loaded = self.memory.load(offset);
let ((high_h, low_h), (high_l, low_l)) = loaded.split_into_u64_le();
to_hash.append(low_h);
to_hash.append(high_h);
to_hash.append(low_l);
to_hash.append(high_l);

offset += 32;
};
offset
}

Expand Down
18 changes: 8 additions & 10 deletions crates/evm/src/memory.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,12 @@ pub(crate) impl InternalMemoryMethods of InternalMemoryTrait {
fn load_aligned_words(
ref self: Memory, mut chunk_index: usize, final_chunk: usize, ref elements: Array<u8>
) {
while chunk_index != final_chunk {
let value = self.items.get(chunk_index.into());
// Pushes 16 items to `elements`
helpers::split_word_128(value.into(), ref elements);
chunk_index += 1;
}
for i in chunk_index
..final_chunk {
let value = self.items.get(i.into());
// Pushes 16 items to `elements`
helpers::split_word_128(value.into(), ref elements);
};
}

/// Loads a `u256` element from the memory chunk at a specified offset.
Expand Down Expand Up @@ -810,10 +810,8 @@ mod tests {
memory.load_n_internal(16, ref results, 0);

assert(results.len() == 16, 'error');
let mut i = 0;
while i != results.len() {
assert(*results[i] == 0xFF, 'byte value loaded not correct');
i += 1;
for result in results {
assert(result == 0xFF, 'byte value loaded not correct');
}
}

Expand Down
11 changes: 5 additions & 6 deletions crates/evm/src/precompiles.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,11 @@ pub const FIRST_ROLLUP_PRECOMPILE_ADDRESS: u256 = 0x100;
/// * `Set<EthAddress>` - A set containing all Ethereum precompile addresses.
pub fn eth_precompile_addresses() -> Set<EthAddress> {
let mut precompile_addresses: Array<EthAddress> = array![];
//TODO(2.8) use range operator
let mut i = FIRST_ETHEREUM_PRECOMPILE_ADDRESS;
while i <= LAST_ETHEREUM_PRECOMPILE_ADDRESS {
precompile_addresses.append(i.try_into().unwrap());
i = i + 1;
};
for i in FIRST_ETHEREUM_PRECOMPILE_ADDRESS
..LAST_ETHEREUM_PRECOMPILE_ADDRESS
+ 0x01 {
precompile_addresses.append(i.try_into().unwrap());
};
SetTrait::from_array(precompile_addresses)
}

Expand Down
17 changes: 5 additions & 12 deletions crates/evm/src/precompiles/blake2f.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -39,22 +39,18 @@ pub impl Blake2f of Precompile {
let mut h: Array<u64> = Default::default();
let mut m: Array<u64> = Default::default();

let mut i = 0;
let mut pos = 4;
while i != 8 {
for _ in 0..8_u8 {
// safe unwrap, because we have made sure of the input length to be 213
h.append(input.slice(pos, 8).from_le_bytes().unwrap());
i += 1;
pos += 8;
};

let mut i = 0;
let mut pos = 68;
while i != 16 {
for _ in 0..16_u8 {
// safe unwrap, because we have made sure of the input length to be 213
m.append(input.slice(pos, 8).from_le_bytes().unwrap());
i += 1;
pos += 8;
pos += 8
};

let mut t: Array<u64> = Default::default();
Expand All @@ -68,12 +64,9 @@ pub impl Blake2f of Precompile {

let mut return_data: Array<u8> = Default::default();

let mut i = 0;
while i != res.len() {
let bytes = (*res[i]).to_le_bytes_padded();
for result in res {
let bytes = (*result).to_le_bytes_padded();
return_data.append_span(bytes);

i += 1;
};

Result::Ok((gas, return_data.span()))
Expand Down
2 changes: 1 addition & 1 deletion crates/evm/src/precompiles/ec_operations/ec_mul.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ fn get_bits_little(s: u256) -> Array<felt252> {
fn ec_mul_inner(pt: (u384, u384), mut bits: Array<felt252>) -> Option<(u384, u384)> {
let (mut temp_x, mut temp_y) = pt;
let mut result: Option<(u384, u384)> = Option::None;
while let Option::Some(bit) = bits.pop_front() {
for bit in bits {
if bit != 0 {
match result {
Option::Some((xr, yr)) => result = ec_safe_add(temp_x, temp_y, xr, yr),
Expand Down
7 changes: 2 additions & 5 deletions crates/evm/src/stack.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,8 @@ impl StackImpl of StackTrait {
fn pop_n(ref self: Stack, mut n: usize) -> Result<Array<u256>, EVMError> {
ensure(!(n > self.len()), EVMError::StackUnderflow)?;
let mut popped_items = ArrayTrait::<u256>::new();
while n != 0 {
for _ in 0..n {
popped_items.append(self.pop().unwrap());
n -= 1;
};
Result::Ok(popped_items)
}
Expand Down Expand Up @@ -346,11 +345,9 @@ mod tests {
fn test_should_fail_when_overflow() {
// Given
let mut stack = StackTrait::new();
let mut i = 0;

// When
while i != constants::STACK_MAX_DEPTH {
i += 1;
for _ in 0..constants::STACK_MAX_DEPTH {
stack.push(1).unwrap();
};

Expand Down
2 changes: 1 addition & 1 deletion crates/evm/src/state.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl StateChangeLogImpl<T, +Drop<T>, +Copy<T>> of StateChangeLogTrait<T> {
fn clone(ref self: StateChangeLog<T>) -> StateChangeLog<T> {
let mut cloned_changes = Default::default();
let mut keyset_span = self.keyset.to_span();
while let Option::Some(key) = keyset_span.pop_front() {
for key in keyset_span {
let value = self.changes.get(*key).deref();
cloned_changes.insert(*key, NullableTrait::new(value));
};
Expand Down
Loading

0 comments on commit c645fee

Please sign in to comment.