Fully implement PUSHn

[rrtoledo]

Description

We intend here to support PUSHn with bytecode length inferior to n.

Issue Link

Issue #633
Linked to specs issue privacy-scaling-explorations/zkevm-specs#463

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Contents

• Update push.rs to support bytecode of size smaller than n
• Update stop.rs similarly

Rationale

In pushn, "selectors" are switches to enable or disable lookup arguments on the bytcode bytes as well as to check whether these are non-null if needs be. They directly depend on "n" of PUSHn.
We add to these "selectors", "counters" which have similar role but are a subset of them depending on the bytecode length. We added these extra switches to be able to easily check the various identities at the risk of some redundancy.

To support these changes, modifications in stop.py also had to be done. More precisely, the check "is_out_of_range had to be extended to support small bytecodes.

How Has This Been Tested?

Extra tests have been added in push.rs (only).

[rrtoledo]

I added a commented num_padding in assign_exec_step as I thought it could be nice to see the parallel withconfigure. Do you think it's worth keeping it or that I should remove it?

[ed255]

I added a commented num_padding in assign_exec_step as I thought it could be nice to see the parallel withconfigure. Do you think it's worth keeping it or that I should remove it?

I didn't see this comment, maybe you didn't push it?

[rrtoledo]

I added a commented num_padding in assign_exec_step as I thought it could be nice to see the parallel withconfigure. Do you think it's worth keeping it or that I should remove it?

I didn't see this comment, maybe you didn't push it?

It is line 198 and 202

        let is_out_of_bound = code_size_left < n;
        // let mut num_padding = 32 - n;
        let mut out_of_bound_padding = 0;
        if is_out_of_bound {
            out_of_bound_padding = n - code_size_left + 1;
            // num_padding += out_of_bound_padding
        };

[ed255]

It is line 198 and 202

        let is_out_of_bound = code_size_left < n;
        // let mut num_padding = 32 - n;
        let mut out_of_bound_padding = 0;
        if is_out_of_bound {
            out_of_bound_padding = n - code_size_left + 1;
            // num_padding += out_of_bound_padding
        };

Oh sorry, for some reason I was thinking it would be in the bus-mapping and I didn't see any changed file there, my bad!
I think the comment helps to follow along! Alternatively you could remove those commented lines and in the circuit add

let out_of_bound_padding = is_out_of_range.expr() * (num_pushed.clone() - code_size_left + 1.expr());
let num_padding = 32.expr() - num_pushed.clone() + out_of_bound_padding;

So that the circuit explicitly matches the logic in the assignment for out_of_bound_padding. But feel free to ignore this :)

[rrtoledo]

It is line 198 and 202

        let is_out_of_bound = code_size_left < n;
        // let mut num_padding = 32 - n;
        let mut out_of_bound_padding = 0;
        if is_out_of_bound {
            out_of_bound_padding = n - code_size_left + 1;
            // num_padding += out_of_bound_padding
        };

Oh sorry, for some reason I was thinking it would be in the bus-mapping and I didn't see any changed file there, my bad! I think the comment helps to follow along! Alternatively you could remove those commented lines and in the circuit add

let out_of_bound_padding = is_out_of_range.expr() * (num_pushed.clone() - code_size_left + 1.expr());
let num_padding = 32.expr() - num_pushed.clone() + out_of_bound_padding;

So that the circuit explicitly matches the logic in the assignment for out_of_bound_padding. But feel free to ignore this :)

It was originally not commented but after some changes it wasn't used any longer so I had to either remove or comment it :)

[ed255]

LGTM! The implementation looks nice and clear! It's really nice that you keep the code well commented :)

I've left a few suggestions regarding variable name consistency with the specs, please take a look! Since they are minor comments I'm approving already.

[lightsing]

General OK, several code style fix needed, like pu/pa, maybe full name is better.
Also some explicit type annotation found, is it necessary?

[han0110]

LGTM!