kkrt-labs / kakarot-ssj

Kakarot zkEVM - rewrite in the latest version of Cairo
https://www.kakarot.org
MIT License
137 stars 83 forks source link

dev: replace iterations on collections with `while` to `for` loops #978

Closed enitrat closed 1 month ago

enitrat commented 2 months ago

For loops being relatively recent, we're not using them everywhere we should in the codebase.

if you do a global search for while i, you will encounter several instances like:

        let mut i = 0;
        while i != self.len() {
            arr.append(*self[i]);
            i += 1;
        };

where we instanciate a loop counter and increase it manually just to loop through a collection.

Instead, we could simply write:

        for item in self {
            arr.append(*item);
        };

This is not applicable to the ByteArray type, which can not be converted to an iterator.

If the loop requires tracking of the internal index, or is not iterating on a collection but on a range of values, like

     while i != chunk_counts {
        let mut value: u256 = (*input[i]).into();
        unpack_chunk(value, 31, ref res);
        i += 1;
    };

we can use a for loop with the range operator:

     for i in 0..chunk_counts {
        let mut value: u256 = (*input[i]).into();
        unpack_chunk(value, 31, ref res);
    };

If the instance you encounter cannot be refactored to use a for loop, keep it as it currently is.

BenFaruna commented 2 months ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hello, my name is Benjamin and I am a software engineer with expertise in blockchain development. I have experience working with cairo and I have written smart contracts for different use cases in cairo.

How I plan on tackling this issue

  1. I would identify the various parts of the codebase where while loop is used.
  2. Refactor while loops into for loops for parts where a for loop is more ideal.
  3. Reach out to the team on the while loops that were not changed to receive feedback.
  4. Make final PR

This should take approximately 2 to 3 days.

fabrobles92 commented 2 months ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

My name is Fabricio Robles, I work as a full stack dev since 3 years ago. Also I have been involved in web3 where I have contributed successfully to projects like dojo engine, Concrete and this project as well creating some testings. I have experience working with Rust, Cairo, Go, Python and Javascript, I am sure that experience would help me delivering a quality solution for this project

How I plan on tackling this issue

I estimate this would take me from 1 - 2 days