chore(blockifier): create func keccak_base to delete code duplication

[avivg-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[avivg-starkware]

• chore(blockifier): create func keccak_base to delete code duplication #2198 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[github-actions]

Artifacts upload triggered. View details here

[codecov]

Codecov Report

Attention: Patch coverage is 84.78261% with 7 lines in your changes missing coverage. Please review.

Project coverage is 68.91%. Comparing base (e3165c4) to head (ff30a90).
Report is 610 commits behind head on main.

Files with missing lines	Patch %	Lines
.../blockifier/src/execution/syscalls/syscall_base.rs	87.17%	4 Missing and 1 partial ⚠️
crates/blockifier/src/execution/syscalls/mod.rs	71.42%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2198       +/-   ##
===========================================
+ Coverage   40.10%   68.91%   +28.81%     
===========================================
  Files          26      108       +82     
  Lines        1895    13982    +12087     
  Branches     1895    13982    +12087     
===========================================
+ Hits          760     9636     +8876     
- Misses       1100     3934     +2834     
- Partials       35      412      +377     

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

[Yoni-Starkware]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @avi-starkware, @avivg-starkware, and @meship-starkware)

---

crates/blockifier/src/execution/syscalls/mod.rs line 666 at r1 (raw file):

    let gas_cost = n_rounds_as_u64 * syscall_handler.context.gas_costs().keccak_round_cost_gas_cost;
    if gas_cost > *remaining_gas {
        let out_of_gas_error =

Try to share this code as well (in the same base func)

Code quote:

    let mut state = [0u64; 25];
    for chunk in data.chunks(KECCAK_FULL_RATE_IN_WORDS) {
        for (i, val) in chunk.iter().enumerate() {
            state[i] ^= val.to_u64().ok_or_else(|| SyscallExecutionError::InvalidSyscallInput {
                input: **val,
                info: String::from("Invalid input for the keccak syscall."),
            })?;
        }
        keccak::f1600(&mut state)
    }

[Yoni-Starkware]

Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @avi-starkware, @avivg-starkware, and @meship-starkware)

---

crates/blockifier/src/execution/native/syscall_handler.rs line 578 at r1 (raw file):

        match syscall_base::keccak_base(self.context, input_length, remaining_gas) {
            Ok(_) => (),
            Err(e) => return Err(self.handle_error(remaining_gas, e.into())),

Can be done in one line?

Code quote:

            Ok(_) => (),
            Err(e) => return Err(self.handle_error(remaining_gas, e.into())),

[github-actions]

Artifacts upload workflows:

• Started at 2024-11-27T12:10:17Z
• Started at 2024-11-27T12:42:54Z
• Started at 2024-11-27T13:27:21Z
• Started at 2024-11-27T15:26:10Z

[avivg-starkware]

Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @avi-starkware, @meship-starkware, and @Yoni-Starkware)

---

crates/blockifier/src/execution/native/syscall_handler.rs line 578 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Can be done in one line?

changed according to other changes. WDYT?

---

crates/blockifier/src/execution/syscalls/mod.rs line 666 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Try to share this code as well (in the same base func)

I moved the conversion logic to be made before hand. please tell me if this implementation looks OK to you. thanks

[Yoni-Starkware]

Reviewed all commit messages.
Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @avi-starkware, @avivg-starkware, and @meship-starkware)

---

crates/blockifier/src/execution/syscalls/syscall_base.rs line 217 at r3 (raw file):

        &mut self,
        input: &[u64],
        input_length: usize,

No need to pass the length - take it from the array.

[Yoni-Starkware]

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @avi-starkware, @avivg-starkware, and @meship-starkware)

---

crates/blockifier/src/execution/syscalls/syscall_base.rs line 214 at r3 (raw file):

    }

    pub fn keccak_base(

Rename and move it above finalize

Suggestion:

    pub fn finalize(&mut self) {
        self.context
            .revert_infos
            .0
            .last_mut()
            .expect("Missing contract revert info.")
            .original_values = std::mem::take(&mut self.original_values);
    }

    pub fn keccak(

[meship-starkware]

Reviewed 1 of 3 files at r1, 2 of 3 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @avi-starkware, @avivg-starkware, and @Yoni-Starkware)

[Yoni-Starkware]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @avi-starkware and @avivg-starkware)

---

crates/blockifier/src/execution/syscalls/syscall_base.rs line 226 at r3 (raw file):

                error_data: vec![
                    Felt::from_hex(INVALID_INPUT_LENGTH_ERROR)
                        .map_err(SyscallExecutionError::from)?,

You can use .expect("Failed to parse INVALID_INPUT_LENGTH_ERROR hex string")

---

crates/blockifier/src/execution/syscalls/syscall_base.rs line 237 at r3 (raw file):

        if gas_cost > *remaining_gas {
            let out_of_gas_error =
                Felt::from_hex(OUT_OF_GAS_ERROR).map_err(SyscallExecutionError::from)?;

Same

Code quote:

Felt::from_hex(OUT_OF_GAS_ERROR).map_err(SyscallExecutionError::from)?;

[Yoni-Starkware]

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @avi-starkware and @avivg-starkware)

---

crates/blockifier/src/execution/native/syscall_handler.rs line 460 at r4 (raw file):

                lo: u128::from(state[0]) | (u128::from(state[1]) << 64),
            }),
            Err(e) => Err(self.handle_error(remaining_gas, e.into())),

I think the into is not necessary

Suggestion:

Err(err) => Err(self.handle_error(remaining_gas, err)),

[meship-starkware]

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @avi-starkware and @avivg-starkware)

[avivg-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @avi-starkware and @Yoni-Starkware)

---

crates/blockifier/src/execution/native/syscall_handler.rs line 460 at r4 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

I think the into is not necessary

yes indeed, done.

---

crates/blockifier/src/execution/syscalls/syscall_base.rs line 214 at r3 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Rename and move it above finalize

Done.

---

crates/blockifier/src/execution/syscalls/syscall_base.rs line 217 at r3 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

No need to pass the length - take it from the array.

Done.

---

crates/blockifier/src/execution/syscalls/syscall_base.rs line 226 at r3 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

You can use .expect("Failed to parse INVALID_INPUT_LENGTH_ERROR hex string")

Done

---

crates/blockifier/src/execution/syscalls/syscall_base.rs line 237 at r3 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Same

Done

[Yoni-Starkware]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @avi-starkware)