implement Blake2s opcode in runner #1927

ohad-nir-starkware · 2025-01-26T18:46:15Z

Blake2s opcode runner

Description

Adding the opcode Blake2s to the VM.
Expects op0 to be a pointer to a sequence of 8 felts and and op1 to be a pointer to a sequence of 16 felts.
Said felts should represent u32 integers, i.e. have value of at most 2**32-1.
The 8 felts of op0 represent a state and the 16 felts of op1 represent a message.
dst should hold the value of the counter.
The "output" consists of 8 felts representing u32 numbers of the output of the (non last block) Blake2s compression.
ap should store a pointer, it points to a sequence of 8 cells which each should either be uninitialised or already contain a value matching that of the output at the same index.
The opcode inserts the aforementioned output into the 8 cells [[ap]], [[ap]+1], ... [[ap]+7] (and yields an error if one of said cells already contains a value differing from the output).

Currently Blake2s has opcode_num 8, meaning encoded_instr can use all 64 bits and InstructionNonZeroHighBits is no longer needed as an error.

The motivation is that it has been decided at Starkware that Blake2s is to be implemented at an opcode, thus it needs to be supported by the runner.

Checklist

Linked to Github Issue
Unit tests added
Integration tests added.
This change requires new documentation.
- Documentation has been added/updated.
- CHANGELOG has been updated.

This change is

github-actions · 2025-01-26T18:56:07Z

**Hyper Thereading Benchmark results**




hyperfine -r 2 -n "hyper_threading_main threads: 1" 'RAYON_NUM_THREADS=1 ./hyper_threading_main' -n "hyper_threading_pr threads: 1" 'RAYON_NUM_THREADS=1 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 1
  Time (mean ± σ):     32.573 s ±  0.668 s    [User: 31.811 s, System: 0.760 s]
  Range (min … max):   32.100 s … 33.045 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 1
  Time (mean ± σ):     32.014 s ±  0.132 s    [User: 31.230 s, System: 0.782 s]
  Range (min … max):   31.920 s … 32.107 s    2 runs
 
Summary
  hyper_threading_pr threads: 1 ran
    1.02 ± 0.02 times faster than hyper_threading_main threads: 1




hyperfine -r 2 -n "hyper_threading_main threads: 2" 'RAYON_NUM_THREADS=2 ./hyper_threading_main' -n "hyper_threading_pr threads: 2" 'RAYON_NUM_THREADS=2 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 2
  Time (mean ± σ):     17.815 s ±  0.039 s    [User: 31.151 s, System: 0.802 s]
  Range (min … max):   17.788 s … 17.843 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 2
  Time (mean ± σ):     17.771 s ±  0.050 s    [User: 31.102 s, System: 0.785 s]
  Range (min … max):   17.735 s … 17.806 s    2 runs
 
Summary
  hyper_threading_pr threads: 2 ran
    1.00 ± 0.00 times faster than hyper_threading_main threads: 2




hyperfine -r 2 -n "hyper_threading_main threads: 4" 'RAYON_NUM_THREADS=4 ./hyper_threading_main' -n "hyper_threading_pr threads: 4" 'RAYON_NUM_THREADS=4 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 4
  Time (mean ± σ):     12.377 s ±  0.146 s    [User: 43.841 s, System: 0.935 s]
  Range (min … max):   12.274 s … 12.480 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 4
  Time (mean ± σ):     12.722 s ±  0.186 s    [User: 43.987 s, System: 0.924 s]
  Range (min … max):   12.590 s … 12.853 s    2 runs
 
Summary
  hyper_threading_main threads: 4 ran
    1.03 ± 0.02 times faster than hyper_threading_pr threads: 4




hyperfine -r 2 -n "hyper_threading_main threads: 6" 'RAYON_NUM_THREADS=6 ./hyper_threading_main' -n "hyper_threading_pr threads: 6" 'RAYON_NUM_THREADS=6 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 6
  Time (mean ± σ):     12.293 s ±  0.065 s    [User: 44.101 s, System: 0.966 s]
  Range (min … max):   12.247 s … 12.338 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 6
  Time (mean ± σ):     12.169 s ±  0.190 s    [User: 44.231 s, System: 0.982 s]
  Range (min … max):   12.035 s … 12.303 s    2 runs
 
Summary
  hyper_threading_pr threads: 6 ran
    1.01 ± 0.02 times faster than hyper_threading_main threads: 6




hyperfine -r 2 -n "hyper_threading_main threads: 8" 'RAYON_NUM_THREADS=8 ./hyper_threading_main' -n "hyper_threading_pr threads: 8" 'RAYON_NUM_THREADS=8 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 8
  Time (mean ± σ):     12.062 s ±  0.097 s    [User: 44.396 s, System: 1.006 s]
  Range (min … max):   11.993 s … 12.130 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 8
  Time (mean ± σ):     11.901 s ±  0.020 s    [User: 44.726 s, System: 0.995 s]
  Range (min … max):   11.886 s … 11.915 s    2 runs
 
Summary
  hyper_threading_pr threads: 8 ran
    1.01 ± 0.01 times faster than hyper_threading_main threads: 8




hyperfine -r 2 -n "hyper_threading_main threads: 16" 'RAYON_NUM_THREADS=16 ./hyper_threading_main' -n "hyper_threading_pr threads: 16" 'RAYON_NUM_THREADS=16 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 16
  Time (mean ± σ):     11.769 s ±  0.060 s    [User: 44.677 s, System: 1.054 s]
  Range (min … max):   11.727 s … 11.811 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 16
  Time (mean ± σ):     12.005 s ±  0.274 s    [User: 44.559 s, System: 1.064 s]
  Range (min … max):   11.812 s … 12.199 s    2 runs
 
Summary
  hyper_threading_main threads: 16 ran
    1.02 ± 0.02 times faster than hyper_threading_pr threads: 16

codecov · 2025-01-26T19:59:25Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.37%. Comparing base (4bd3a02) to head (09c9b39).

Additional details and impacted files

@@                  Coverage Diff                  @@
##           ohadn/get_u32_range    #1927    +/-   ##
=====================================================
  Coverage                96.36%   96.37%            
=====================================================
  Files                      102      102            
  Lines                    41196    41345   +149     
=====================================================
+ Hits                     39700    39846   +146     
- Misses                    1496     1499     +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2025-01-27T10:35:08Z

Benchmark Results for unmodified programs 🚀

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_factorial`	2.531 ± 0.080	2.485	2.754	1.01 ± 0.03
`head big_factorial`	2.499 ± 0.028	2.467	2.572	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_fibonacci`	2.440 ± 0.015	2.429	2.479	1.00
`head big_fibonacci`	2.448 ± 0.062	2.410	2.608	1.00 ± 0.03

Command	Mean [s]	Min [s]	Max [s]	Relative
`base blake2s_integration_benchmark`	9.108 ± 0.040	9.020	9.166	1.00
`head blake2s_integration_benchmark`	9.142 ± 0.152	8.983	9.477	1.00 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base compare_arrays_200000`	2.582 ± 0.018	2.553	2.609	1.00
`head compare_arrays_200000`	2.583 ± 0.060	2.527	2.746	1.00 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base dict_integration_benchmark`	1.716 ± 0.056	1.674	1.846	1.02 ± 0.04
`head dict_integration_benchmark`	1.686 ± 0.037	1.666	1.789	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base field_arithmetic_get_square_benchmark`	1.437 ± 0.019	1.421	1.487	1.00
`head field_arithmetic_get_square_benchmark`	1.439 ± 0.033	1.414	1.530	1.00 ± 0.03

Command	Mean [s]	Min [s]	Max [s]	Relative
`base integration_builtins`	9.244 ± 0.090	9.162	9.421	1.01 ± 0.02
`head integration_builtins`	9.183 ± 0.107	9.072	9.372	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base keccak_integration_benchmark`	9.655 ± 0.407	9.360	10.680	1.02 ± 0.04
`head keccak_integration_benchmark`	9.433 ± 0.090	9.332	9.572	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base linear_search`	2.590 ± 0.040	2.552	2.664	1.01 ± 0.02
`head linear_search`	2.557 ± 0.019	2.524	2.582	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_cmp_and_pow_integration_benchmark`	1.739 ± 0.023	1.712	1.795	1.00
`head math_cmp_and_pow_integration_benchmark`	1.747 ± 0.021	1.727	1.796	1.00 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_integration_benchmark`	1.677 ± 0.013	1.666	1.707	1.00
`head math_integration_benchmark`	1.694 ± 0.010	1.682	1.717	1.01 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base memory_integration_benchmark`	1.446 ± 0.031	1.413	1.524	1.02 ± 0.02
`head memory_integration_benchmark`	1.422 ± 0.012	1.405	1.443	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base operations_with_data_structures_benchmarks`	1.794 ± 0.016	1.775	1.820	1.00
`head operations_with_data_structures_benchmarks`	1.834 ± 0.049	1.797	1.939	1.02 ± 0.03

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base pedersen`	585.9 ± 3.9	581.5	594.0	1.00
`head pedersen`	590.0 ± 3.2	586.4	595.4	1.01 ± 0.01

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base poseidon_integration_benchmark`	698.4 ± 17.4	681.5	744.4	1.01 ± 0.03
`head poseidon_integration_benchmark`	688.5 ± 7.4	676.7	698.9	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base secp_integration_benchmark`	2.058 ± 0.023	2.043	2.121	1.00
`head secp_integration_benchmark`	2.065 ± 0.014	2.042	2.092	1.00 ± 0.01

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base set_integration_benchmark`	699.9 ± 2.5	695.0	703.3	1.03 ± 0.01
`head set_integration_benchmark`	680.7 ± 2.8	674.5	683.8	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base uint256_integration_benchmark`	5.087 ± 0.107	5.002	5.312	1.01 ± 0.03
`head uint256_integration_benchmark`	5.047 ± 0.078	4.961	5.183	1.00

DavidLevitGurevich · 2025-01-30T09:43:24Z

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo

+
+// Tests the Blake2s opcode runner using a preexisting implementation within the repo as reference.
+// The initial state, a random message of 68 bytes and counter are used as input.
+// Both the opcode and the reference implementation are run on said inputs and outputs are compared.


I don't understand this sentence

did you mean said -> same?

DavidLevitGurevich · 2025-01-30T10:39:32Z

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo

+// An instruction encoding is built from offsets -5, -4, -3 and flags which are all 0 except for
+// those denoting uses of fp as the base for operand addresses and flag_opcode_blake (16th flag).
+// The instruction is then written to [pc] and the runner is forced to execute Blake2s.
+func force_blake2s_non_last_block_opcode(


just opcode_blake2s or execute_blake2s or blake2s or run_blake2s

Stavbe

Reviewable status: 0 of 11 files reviewed, 9 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @ohad-nir-starkware, @Oppen, @pefontana, and @YairVaknin-starkware)

vm/src/vm/vm_core.rs line 1007 at r2 (raw file):

        for (i, x) in res_cow.iter().enumerate() {
            let le_digits = x.clone().into_owned().to_le_digits();
            if le_digits[0] >= (1 << 32)

This conversion to u32 can be moved to a function and also called from handle_blake_2s

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 46 at r2 (raw file):

    assert blake2s_ptr[7] = 0x5BE0CD19;
    static_assert STATE_SIZE_FELTS == 8;
    let blake2s_ptr = blake2s_ptr + STATE_SIZE_FELTS;

Another variable with the same name on purpose?

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 50 at r2 (raw file):

    let (cairo_output) = blake2s_inner{range_check_ptr=range_check_ptr, blake2s_ptr=blake2s_ptr}(data=random_message, n_bytes=INPUT_BLOCK_BYTES+4, counter=COUNTER);

    let relevant_output_start = blake2s_ptr_start+INPUT_BLOCK_FELTS+2+STATE_SIZE_FELTS;

2?

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 128 at r2 (raw file):

    let vm_output_start = cast([ap], felt*);
    return vm_output_start;
}

add new line

vm/src/vm/decoding/decoder.rs line 8 at r2 (raw file):

};

//          opcode|ap_update|pc_update|res_logic|op1_src|op0_reg|dst_reg

opcode_extension | opcode |
16 15

vm/src/vm/decoding/decoder.rs line 101 at r2 (raw file):

    let opcode_extension = match opcode_extension_num {
        0 => OpcodeExtension::Stone,
        _ => {

1 ?

vm/src/vm/decoding/decoder.rs line 102 at r2 (raw file):

        0 => OpcodeExtension::Stone,
        _ => {
            if opcode != Opcode::NOp {

another assertion:
-> op1 src can be only 2/4
res -> op1
pc_update-> regular

DavidLevitGurevich

Reviewed 1 of 3 files at r2.
Reviewable status: 1 of 11 files reviewed, 14 unresolved discussions (waiting on @fmoletta, @gabrielbosio, @igaray, @juanbono, @ohad-nir-starkware, @Oppen, @pefontana, @Stavbe, and @YairVaknin-starkware)

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 46 at r2 (raw file):

Previously, Stavbe wrote…

Another variable with the same name on purpose?

this is a standard pattern

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 9 at r2 (raw file):

// Tests the Blake2s opcode runner using a preexisting implementation within the repo as reference.
// The initial state, a random message of 68 bytes and counter are used as input.

why are you using 68 bytes (more than a single block) if you are testing only one block?

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 48 at r2 (raw file):

    let blake2s_ptr = blake2s_ptr + STATE_SIZE_FELTS;

    let (cairo_output) = blake2s_inner{range_check_ptr=range_check_ptr, blake2s_ptr=blake2s_ptr}(data=random_message, n_bytes=INPUT_BLOCK_BYTES+4, counter=COUNTER);

why do you compare to blake2s_inner and not to blake2s_compress?

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 58 at r2 (raw file):

    );

    tempvar check_nonempty = vm_output_start[0];

does it really check that it's nonempty? if you don't initialize it on porpuse does it shout?

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 67 at r2 (raw file):

    tempvar check_nonempty = vm_output_start[7];

    assert vm_output_start[0] = relevant_output_start[0];

you can avoid this by passing the vm_output_start as input to run_blake2s

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 88 at r2 (raw file):

// those denoting uses of fp as the base for operand addresses and flag_opcode_blake (16th flag).
// The instruction is then written to [pc] and the runner is forced to execute Blake2s.
func run_blake2s(

you can have the is_last_block flag as input, place 2 dws and jump to the correct one

ohad-nir-starkware

Reviewable status: 1 of 11 files reviewed, 13 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, @pefontana, @Stavbe, and @YairVaknin-starkware)

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 50 at r2 (raw file):

Previously, Stavbe wrote…

2?

blake2s_inner writes t0 and f0 into the blake_ptr segment

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 58 at r2 (raw file):

Previously, DavidLevitGurevich wrote…

does it really check that it's nonempty? if you don't initialize it on porpuse does it shout?

yes.

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 67 at r2 (raw file):

Previously, DavidLevitGurevich wrote…

you can avoid this by passing the vm_output_start as input to run_blake2s

avoid what?

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 88 at r2 (raw file):

Previously, DavidLevitGurevich wrote…

you can have the is_last_block flag as input, place 2 dws and jump to the correct one

thanks, maybe that's a good idea in the blake_last_block PR (#1932)
does that mean you want both tests to be done in one cairo file?

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 128 at r2 (raw file):

Previously, Stavbe wrote…

add new line

Done.

vm/src/vm/decoding/decoder.rs line 8 at r2 (raw file):

Previously, Stavbe wrote…

opcode_extension | opcode |
16 15

Done.

vm/src/vm/decoding/decoder.rs line 101 at r2 (raw file):

Previously, Stavbe wrote…

1 ?

Done.

vm/src/vm/decoding/decoder.rs line 102 at r2 (raw file):

Previously, Stavbe wrote…

another assertion:
-> op1 src can be only 2/4
res -> op1
pc_update-> regular

Done.

ohad-nir-starkware

Reviewable status: 0 of 11 files reviewed, 13 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, @pefontana, @Stavbe, and @YairVaknin-starkware)

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 9 at r2 (raw file):

Previously, DavidLevitGurevich wrote…

why are you using 68 bytes (more than a single block) if you are testing only one block?

when using blake2s_inner with 64 or less bytes, it calls blake2s_last_block

DavidLevitGurevich

Reviewed 1 of 6 files at r3.
Reviewable status: 1 of 11 files reviewed, 12 unresolved discussions (waiting on @fmoletta, @gabrielbosio, @igaray, @juanbono, @ohad-nir-starkware, @Oppen, @pefontana, @Stavbe, and @YairVaknin-starkware)

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 9 at r2 (raw file):

Previously, ohad-nir-starkware (Ohad Nir) wrote…

when using blake2s_inner with 64 or less bytes, it calls blake2s_last_block

I see, so it comes back to my other comment:
why do you compare to blake2s_inner and not to blake2s_compress?

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 67 at r2 (raw file):

Previously, ohad-nir-starkware (Ohad Nir) wrote…

avoid what?

avoid copying data from one array to another

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 88 at r2 (raw file):

Previously, ohad-nir-starkware (Ohad Nir) wrote…

thanks, maybe that's a good idea in the blake_last_block PR (#1932)
does that mean you want both tests to be done in one cairo file?

yes, I think that your current solution has lots of code duplication

ohad-nir-starkware

Reviewable status: 1 of 11 files reviewed, 6 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, @pefontana, @Stavbe, and @YairVaknin-starkware)

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 9 at r2 (raw file):

Previously, DavidLevitGurevich wrote…

I see, so it comes back to my other comment:
why do you compare to blake2s_inner and not to blake2s_compress?

Done.

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 48 at r2 (raw file):

Previously, DavidLevitGurevich wrote…

why do you compare to blake2s_inner and not to blake2s_compress?

Done.

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 67 at r2 (raw file):

Previously, DavidLevitGurevich wrote…

avoid copying data from one array to another

Done.

vm/src/vm/vm_core.rs line 1007 at r2 (raw file):

Previously, Stavbe wrote…

This conversion to u32 can be moved to a function and also called from handle_blake_2s

Done.

Stavbe

Reviewable status: 0 of 11 files reviewed, 6 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @ohad-nir-starkware, @Oppen, @pefontana, and @YairVaknin-starkware)

vm/src/vm/decoding/decoder.rs line 8 at r2 (raw file):

Previously, ohad-nir-starkware (Ohad Nir) wrote…

Done.

16 ?

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 164 at r4 (raw file):

    let vm_output = cast([ap], felt*);
    return vm_output;
}

new line

ohad-nir-starkware

Reviewable status: 0 of 11 files reviewed, 6 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, @pefontana, @Stavbe, and @YairVaknin-starkware)

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 164 at r4 (raw file):

Previously, Stavbe wrote…

new line

Done.

vm/src/vm/decoding/decoder.rs line 8 at r2 (raw file):

Previously, Stavbe wrote…

16 ?

no 16 yet, only when introducing blake_last_block

DavidLevitGurevich

Reviewed 3 of 5 files at r5.
Reviewable status: 3 of 11 files reviewed, 4 unresolved discussions (waiting on @fmoletta, @gabrielbosio, @igaray, @juanbono, @ohad-nir-starkware, @Oppen, @pefontana, @Stavbe, and @YairVaknin-starkware)

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 162 at r5 (raw file):

    dw 9226608988349300731;

    let vm_output = cast([ap], felt*);

why do you need this line?

Code quote:

let vm_output = cast([ap], felt*);

ohad-nir-starkware

Reviewable status: 3 of 11 files reviewed, 4 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, @pefontana, @Stavbe, and @YairVaknin-starkware)

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 162 at r5 (raw file):

Previously, DavidLevitGurevich wrote…

why do you need this line?

seems that the line dw 9226608988349300731; made the compiler revoke the reference to vm_output.
however I can return cast([ap], felt*) directly, without setting to vm_output.

DavidLevitGurevich

Reviewed 1 of 11 files at r1, 1 of 6 files at r3, 6 of 7 files at r7, all commit messages.
Reviewable status: 8 of 11 files reviewed, 7 unresolved discussions (waiting on @fmoletta, @gabrielbosio, @igaray, @juanbono, @ohad-nir-starkware, @Oppen, @pefontana, @Stavbe, and @YairVaknin-starkware)

vm/src/vm/decoding/decoder.rs line 112 at r7 (raw file):

            if (op1_addr != Op1Addr::FP && op1_addr != Op1Addr::AP)
                || res != Res::Op1
                || pc_update != PcUpdate::Regular

you should check also ApUpdate

vm/src/vm/decoding/decoder.rs line 415 at r7 (raw file):

    fn decode_opcode_extension_clash() {
        // opcode_extension|   opcode|ap_update|pc_update|res_logic|op1_src|op0_reg|dst_reg
        //               15| 14 13 12|    11 10|  9  8  7|     6  5|4  3  2|      1|      0

...

vm/src/vm/decoding/decoder.rs line 416 at r7 (raw file):

        // opcode_extension|   opcode|ap_update|pc_update|res_logic|op1_src|op0_reg|dst_reg
        //               15| 14 13 12|    11 10|  9  8  7|     6  5|4  3  2|      1|      0
        //            Blake|     CALL|     Add2|  JumpRel|      Op1|     FP|     FP|     FP

fix this line

Code quote:

 //            Blake|     CALL|     Add2|  JumpRel|      Op1|     FP|     FP|     FP

vm/src/vm/decoding/decoder.rs line 417 at r7 (raw file):

        //               15| 14 13 12|    11 10|  9  8  7|     6  5|4  3  2|      1|      0
        //            Blake|     CALL|     Add2|  JumpRel|      Op1|     FP|     FP|     FP
        //                1   0  0  1      0  0   0  1  0      0  0 0  0  1       0       0

you have op1_imm set, this should be a separate test

DavidLevitGurevich

Reviewed 1 of 7 files at r7.
Reviewable status: 9 of 11 files reviewed, 11 unresolved discussions (waiting on @fmoletta, @gabrielbosio, @igaray, @juanbono, @ohad-nir-starkware, @Oppen, @pefontana, @Stavbe, and @YairVaknin-starkware)

vm/src/vm/vm_core.rs line 462 at r7 (raw file):

    /// op0 is expected to point to a sequence of 8 u32 values (state).
    /// op1 is expected to point to a sequence of 16 u32 values (message).
    /// dst is expected hold the u32 value of the counter.

(t)

vm/src/vm/vm_core.rs line 463 at r7 (raw file):

    /// op1 is expected to point to a sequence of 16 u32 values (message).
    /// dst is expected hold the u32 value of the counter.
    /// ap is expected to point to a sequence of 8 cells each being either unitialised or

[ap]

vm/src/vm/vm_core.rs line 466 at r7 (raw file):

    /// containing the Blake2s compression output at that index.
    /// Deviation from the aforementioned expectations will result in an error.
    /// The instruction will update the ap memory segment with the new state.

The instruction will update the memory segment pointed by [ap] with the new state.

vm/src/vm/vm_core.rs line 484 at r7 (raw file):

        .map_err(|_| VirtualMachineError::Blake2sInvalidOperand(0, 8))?;

        let message = (self.get_u32_range(

: [u32, 16]

Stavbe

Reviewable status: 9 of 11 files reviewed, 11 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @ohad-nir-starkware, @Oppen, @pefontana, and @YairVaknin-starkware)

vm/src/vm/decoding/decoder.rs line 112 at r7 (raw file):

Previously, DavidLevitGurevich wrote…

you should check also ApUpdate

can be 0/2

vm/src/vm/decoding/decoder.rs line 105 at r7 (raw file):

        1 => {
            if opcode != Opcode::NOp {
                return Err(VirtualMachineError::OpcodeExtensionClash(

i thinks this check should be done in one if and send the same error : InvalidBlake2sFlags(flags)

ohad-nir-starkware

Reviewable status: 9 of 11 files reviewed, 11 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, @pefontana, @Stavbe, and @YairVaknin-starkware)

cairo_programs/stwo_exclusive_programs/blake2s_opcode_test.cairo line 88 at r2 (raw file):

Previously, DavidLevitGurevich wrote…

yes, I think that your current solution has lots of code duplication

Done.

vm/src/vm/vm_core.rs line 462 at r7 (raw file):

Previously, DavidLevitGurevich wrote…

(t)

Done.

vm/src/vm/vm_core.rs line 463 at r7 (raw file):

Previously, DavidLevitGurevich wrote…

[ap]

Done.

vm/src/vm/vm_core.rs line 466 at r7 (raw file):

Previously, DavidLevitGurevich wrote…

The instruction will update the memory segment pointed by [ap] with the new state.

Done.

vm/src/vm/vm_core.rs line 484 at r7 (raw file):

Previously, DavidLevitGurevich wrote…

: [u32, 16]

Done.

vm/src/vm/decoding/decoder.rs line 105 at r7 (raw file):

Previously, Stavbe wrote…

i thinks this check should be done in one if and send the same error : InvalidBlake2sFlags(flags)

Done.

vm/src/vm/decoding/decoder.rs line 112 at r7 (raw file):

Previously, Stavbe wrote…

can be 0/2

Done.

vm/src/vm/decoding/decoder.rs line 415 at r7 (raw file):

Previously, DavidLevitGurevich wrote…

...

Done.

vm/src/vm/decoding/decoder.rs line 416 at r7 (raw file):

Previously, DavidLevitGurevich wrote…

fix this line

Done.

vm/src/vm/decoding/decoder.rs line 417 at r7 (raw file):

Previously, DavidLevitGurevich wrote…

you have op1_imm set, this should be a separate test

Done.

ohad-nir-starkware requested review from igaray, Oppen, fmoletta, juanbono, pefontana and gabrielbosio as code owners January 26, 2025 18:46

ohad-nir-starkware self-assigned this Jan 26, 2025

ohad-nir-starkware requested a review from YairVaknin-starkware January 26, 2025 18:46

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch from b3b5246 to 4f6a596 Compare January 26, 2025 18:47

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch from 4f6a596 to 13d5632 Compare January 26, 2025 19:06

ohad-nir-starkware removed the request for review from YairVaknin-starkware January 26, 2025 19:34

ohad-nir-starkware requested a review from YairVaknin-starkware January 27, 2025 07:22

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch from 13d5632 to 6738a48 Compare January 27, 2025 10:12

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch 4 times, most recently from 5d273fc to 912b050 Compare January 29, 2025 09:41

DavidLevitGurevich suggested changes Jan 30, 2025

View reviewed changes

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch 3 times, most recently from d25cef1 to 1c031e4 Compare February 3, 2025 09:13

Stavbe reviewed Feb 3, 2025

View reviewed changes

DavidLevitGurevich reviewed Feb 3, 2025

View reviewed changes

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch from 1c031e4 to de91ec1 Compare February 3, 2025 19:44

ohad-nir-starkware commented Feb 3, 2025

View reviewed changes

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch from de91ec1 to 66999d2 Compare February 3, 2025 19:46

ohad-nir-starkware commented Feb 3, 2025

View reviewed changes

DavidLevitGurevich reviewed Feb 3, 2025

View reviewed changes

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch from 66999d2 to 08a334c Compare February 4, 2025 09:40

ohad-nir-starkware commented Feb 4, 2025

View reviewed changes

Stavbe reviewed Feb 4, 2025

View reviewed changes

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch from 08a334c to 2c08c1a Compare February 4, 2025 10:54

ohad-nir-starkware commented Feb 4, 2025

View reviewed changes

ohad-nir-starkware changed the base branch from main to ohadn/opcode_extension February 4, 2025 10:55

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch from 2c08c1a to 8d9a789 Compare February 4, 2025 11:52

ohad-nir-starkware changed the base branch from ohadn/opcode_extension to ohadn/get_u32_range February 4, 2025 11:53

ohad-nir-starkware force-pushed the ohadn/get_u32_range branch from d220115 to dd1e261 Compare February 4, 2025 12:19

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch from 8d9a789 to 50b3c1b Compare February 4, 2025 12:21

ohad-nir-starkware force-pushed the ohadn/get_u32_range branch from dd1e261 to f355cc7 Compare February 4, 2025 13:55

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch 2 times, most recently from 09d1a17 to 7cbc06f Compare February 4, 2025 19:11

DavidLevitGurevich reviewed Feb 4, 2025

View reviewed changes

ohad-nir-starkware force-pushed the ohadn/get_u32_range branch 3 times, most recently from a4b95e7 to 7cc076a Compare February 5, 2025 07:53

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch 2 times, most recently from b26563b to 246904f Compare February 5, 2025 08:03

ohad-nir-starkware commented Feb 5, 2025

View reviewed changes

DavidLevitGurevich reviewed Feb 5, 2025

View reviewed changes

Stavbe reviewed Feb 5, 2025

View reviewed changes

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch from 246904f to 4490aab Compare February 5, 2025 12:49

ohad-nir-starkware commented Feb 5, 2025

View reviewed changes

ohad-nir-starkware force-pushed the ohadn/get_u32_range branch from 7cc076a to 4bd3a02 Compare February 5, 2025 12:58

implement Blake2s opcode in runner

09c9b39

ohad-nir-starkware force-pushed the ohadn/blake2s_opcode_runner branch from 4490aab to 09c9b39 Compare February 5, 2025 13:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement Blake2s opcode in runner #1927

implement Blake2s opcode in runner #1927

ohad-nir-starkware commented Jan 26, 2025 •

edited

Loading

github-actions bot commented Jan 26, 2025 •

edited

Loading

codecov bot commented Jan 26, 2025 •

edited

Loading

github-actions bot commented Jan 27, 2025 •

edited

Loading

DavidLevitGurevich Jan 30, 2025

DavidLevitGurevich Jan 30, 2025

ohad-nir-starkware Jan 30, 2025

DavidLevitGurevich Jan 30, 2025

ohad-nir-starkware Jan 30, 2025

Stavbe left a comment

DavidLevitGurevich left a comment

ohad-nir-starkware left a comment

ohad-nir-starkware left a comment

DavidLevitGurevich left a comment

ohad-nir-starkware left a comment

Stavbe left a comment

ohad-nir-starkware left a comment

DavidLevitGurevich left a comment

ohad-nir-starkware left a comment

DavidLevitGurevich left a comment

DavidLevitGurevich left a comment

Stavbe left a comment

ohad-nir-starkware left a comment

implement Blake2s opcode in runner #1927

Are you sure you want to change the base?

implement Blake2s opcode in runner #1927

Conversation

ohad-nir-starkware commented Jan 26, 2025 • edited Loading

Blake2s opcode runner

Description

Checklist

github-actions bot commented Jan 26, 2025 • edited Loading

codecov bot commented Jan 26, 2025 • edited Loading

Codecov Report

github-actions bot commented Jan 27, 2025 • edited Loading

DavidLevitGurevich Jan 30, 2025

Choose a reason for hiding this comment

DavidLevitGurevich Jan 30, 2025

Choose a reason for hiding this comment

ohad-nir-starkware Jan 30, 2025

Choose a reason for hiding this comment

DavidLevitGurevich Jan 30, 2025

Choose a reason for hiding this comment

ohad-nir-starkware Jan 30, 2025

Choose a reason for hiding this comment

Stavbe left a comment

Choose a reason for hiding this comment

DavidLevitGurevich left a comment

Choose a reason for hiding this comment

ohad-nir-starkware left a comment

Choose a reason for hiding this comment

ohad-nir-starkware left a comment

Choose a reason for hiding this comment

DavidLevitGurevich left a comment

Choose a reason for hiding this comment

ohad-nir-starkware left a comment

Choose a reason for hiding this comment

Stavbe left a comment

Choose a reason for hiding this comment

ohad-nir-starkware left a comment

Choose a reason for hiding this comment

DavidLevitGurevich left a comment

Choose a reason for hiding this comment

ohad-nir-starkware left a comment

Choose a reason for hiding this comment

DavidLevitGurevich left a comment

Choose a reason for hiding this comment

DavidLevitGurevich left a comment

Choose a reason for hiding this comment

Stavbe left a comment

Choose a reason for hiding this comment

ohad-nir-starkware left a comment

Choose a reason for hiding this comment

ohad-nir-starkware commented Jan 26, 2025 •

edited

Loading

github-actions bot commented Jan 26, 2025 •

edited

Loading

codecov bot commented Jan 26, 2025 •

edited

Loading

github-actions bot commented Jan 27, 2025 •

edited

Loading