chore(blockifier): remove syscall_gas_costs from os_constans #2864

Yonatan-Starkware · 2024-12-22T11:59:40Z

No description provided.

reviewable-StarkWare · 2024-12-22T11:59:45Z

This change is

github-actions · 2024-12-22T11:59:52Z

Artifacts upload workflows:

Yonatan-Starkware · 2024-12-22T12:00:04Z

chore(blockifier): remove syscall_gas_costs from os_constans #2864 👈 (View in Graphite)
chore(blockifier): remove fee_transfer_gas_cost #2858
chore(blockifier): impl deserialize for versioned_constants #2838
chore(blockifier): split builtin gas costs from base gas costs #2514 : 1 other dependent PR (#2818 )
chore(blockifier): add syscalls_gas_costs struct #2359 : 1 other dependent PR (#2501 )
chore(blockifier): add keccak_builtin_gas_cost #2327
chore(blockifier): replace entry_point_gas_cost with initial_budget #2247
feat(blockifier): add get_syscall_gas_cost to versioned constants impl #2188
feat(blockifier): add impl for GasCosts in versioned_constants #2067
main

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

crates/blockifier/src/execution/entry_point.rs

meship-starkware

Dismissed @graphite-app[bot] from a discussion.
Reviewable status: 0 of 14 files reviewed, 1 unresolved discussion (waiting on @noaov1 and @Yonatan-Starkware)

crates/blockifier/src/execution/entry_point.rs line 160 at r2 (raw file):

        ));

        println!("remaining_gas: {}", remaining_gas);

remove before merge

Code quote:

  println!("remaining_gas: {}", remaining_gas);

meship-starkware

Reviewed 9 of 10 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @noaov1 and @Yonatan-Starkware)

crates/blockifier/src/execution/execution_utils.rs line 88 at r2 (raw file):

            }
            update_remaining_gas(remaining_gas, &call_info);
            println!("here with remaining gas: {}", remaining_gas);

same

Code quote:

     println!("here with remaining gas: {}", remaining_gas);

crates/blockifier/resources/versioned_constants_0_13_4.json line 217 at r2 (raw file):

                "builtin_instance_counter": {},
                "n_memory_holes": 0
            },

add this with 0 to all old versioned constans

Code quote:

            "Keccak": {
                "n_steps": 100,
                "builtin_instance_counter": {},
                "n_memory_holes": 0
            },

crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs line 37 at r2 (raw file):

            // 'Out of gas'
            retdata: retdata![felt!["0x4f7574206f6620676173"]],
            gas_consumed: REQUIRED_GAS_STORAGE_READ_WRITE_TEST ,

Why? I am just curious as to why it works without the 70 now

Code quote:

  gas_consumed: REQUIRED_GAS_STORAGE_READ_WRITE_TEST

meship-starkware

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @noaov1 and @Yonatan-Starkware)

crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs line 37 at r2 (raw file):

            // 'Out of gas'
            retdata: retdata![felt!["0x4f7574206f6620676173"]],
            gas_consumed: REQUIRED_GAS_STORAGE_READ_WRITE_TEST ,

please revert when you understand what makes the test fall

Code quote:

        gas_consumed: REQUIRED_GAS_STORAGE_READ_WRITE_TEST

noaov1

Reviewed 9 of 10 files at r1, 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Yonatan-Starkware)

crates/blockifier/src/execution/deprecated_syscalls/mod.rs line 111 at r2 (raw file):

            b"GetTxSignature" => Ok(Self::GetTxSignature),
            b"Keccak" => Ok(Self::Keccak),
            b"KeccakRound" => Ok(Self::KeccakRound),

Please remove

Code quote:

b"KeccakRound" => Ok(Self::KeccakRound),

Yonatan-Starkware · 2024-12-26T07:14:58Z

crates/blockifier/resources/versioned_constants_0_13_4.json line 217 at r2 (raw file):

Previously, meship-starkware (Meshi Peled) wrote…

add this with 0 to all old versioned constans

0 n_steps?

Yonatan-Starkware

Reviewable status: 0 of 20 files reviewed, 5 unresolved discussions (waiting on @graphite-app[bot], @meship-starkware, and @noaov1)

crates/blockifier/src/execution/deprecated_syscalls/mod.rs line 111 at r2 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Please remove

Done.

noaov1

Reviewed 2 of 4 files at r11, 2 of 2 files at r12, 1 of 1 files at r13, 4 of 4 files at r14, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Yonatan-Starkware)

crates/blockifier/src/versioned_constants.rs line 744 at r10 (raw file):

Previously, Yonatan-Starkware wrote…

just in the test.

I think it's better to remove it.

Yonatan-Starkware · 2024-12-29T15:24:16Z

crates/blockifier/src/versioned_constants.rs line 744 at r10 (raw file):

Previously, noaov1 (Noa Oved) wrote…

I think it's better to remove it.

Done.

noaov1

Reviewed 2 of 2 files at r15, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Yonatan-Starkware)

crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs line 22 at r15 (raw file):

use crate::versioned_constants::VersionedConstants;

const SYSCALL_BASE_GAS_COST: u64 = 10000;

And then use:
let syscall_base_gas_cost = gas_costs.base.get_base_gas_cost(SYSCALL_BASE_GAS_COST);

Suggestion:

const SYSCALL_BASE_GAS_COST: &str = "syscall_base_gas_cost";

Yonatan-Starkware · 2025-01-02T09:05:16Z

crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs line 22 at r15 (raw file):

Previously, noaov1 (Noa Oved) wrote…

And then use:
let syscall_base_gas_cost = gas_costs.base.get_base_gas_cost(SYSCALL_BASE_GAS_COST);

but we didn't impeled get_base_gas_cost. Should we?

noaov1

Reviewed 16 of 18 files at r16, 2 of 2 files at r17, 1 of 1 files at r18, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Yonatan-Starkware)

crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs line 22 at r15 (raw file):

Previously, Yonatan-Starkware wrote…

but we didn't impeled get_base_gas_cost. Should we?

Yes

meship-starkware

Reviewed 1 of 4 files at r11, 2 of 2 files at r12, 1 of 1 files at r13, 10 of 18 files at r16, 2 of 2 files at r17, 1 of 1 files at r18, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Yonatan-Starkware)

Yonatan-Starkware · 2025-01-02T16:34:59Z

crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs line 22 at r15 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Yes

Done.

meship-starkware

Reviewed 2 of 2 files at r19, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yonatan-Starkware)

meship-starkware

Reviewed 5 of 18 files at r16, 3 of 3 files at r20, 1 of 1 files at r21, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yonatan-Starkware)

noaov1

Reviewed 1 of 2 files at r19, 1 of 3 files at r20, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Yonatan-Starkware)

crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs line 43 at r21 (raw file):

    // We hit the out of gas error right before executing the syscall.
    let syscall_base_gas_cost = gas_costs.base.get_base_gas_cost(SYSCALL_BASE_GAS_COST);

Will this work? If so, please remove get_base_gas_cost.

Suggestion:

    let syscall_base_gas_cost = gas_costs.base.syscall_base_gas_cost;

Yonatan-Starkware · 2025-01-05T17:38:35Z

crates/blockifier/src/execution/syscalls/syscall_tests/out_of_gas.rs line 43 at r21 (raw file):

Previously, noaov1 (Noa Oved) wrote…

Will this work? If so, please remove get_base_gas_cost.

works. Done.

noaov1

Reviewed 1 of 3 files at r20, 2 of 2 files at r22, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yonatan-Starkware)

noaov1

Reviewed 1 of 3 files at r20, 1 of 1 files at r21.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yonatan-Starkware)

Yonatan-Starkware requested review from meship-starkware and noaov1 December 22, 2024 11:59

Yonatan-Starkware marked this pull request as ready for review December 22, 2024 12:00

Yonatan-Starkware mentioned this pull request Dec 22, 2024

chore(blockifier): impl deserialize for versioned_constants #2838

Merged

Yonatan-Starkware mentioned this pull request Dec 22, 2024

chore(blockifier): remove fee_transfer_gas_cost #2858

Merged

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch from 43571e9 to d5e0738 Compare December 22, 2024 13:29

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_fee_transfer_gas_cost branch from 3c6c184 to cbd78b0 Compare December 22, 2024 13:39

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch 2 times, most recently from 8d3e3f2 to 01e61b3 Compare December 22, 2024 13:53

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_fee_transfer_gas_cost branch from cbd78b0 to 387ef7c Compare December 22, 2024 14:07

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch from 01e61b3 to 42fa457 Compare December 22, 2024 14:08

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_fee_transfer_gas_cost branch from 387ef7c to 5ae0664 Compare December 22, 2024 16:05

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch 2 times, most recently from cd9dca7 to 74046d7 Compare December 22, 2024 16:39

graphite-app bot reviewed Dec 22, 2024

View reviewed changes

crates/blockifier/src/execution/entry_point.rs Outdated Show resolved Hide resolved

meship-starkware requested changes Dec 23, 2024

View reviewed changes

Base automatically changed from yonatank/blockifier/remove_fee_transfer_gas_cost to yonatank/blockifier/impl_deserialize_for_versioned_constants December 24, 2024 08:35

noaov1 requested changes Dec 25, 2024

View reviewed changes

Yonatan-Starkware force-pushed the yonatank/blockifier/impl_deserialize_for_versioned_constants branch 2 times, most recently from 3f2d271 to 380967f Compare December 26, 2024 08:38

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch from 74046d7 to 984a410 Compare December 26, 2024 08:42

Base automatically changed from yonatank/blockifier/impl_deserialize_for_versioned_constants to main-v0.13.4 December 26, 2024 11:46

Yonatan-Starkware commented Dec 26, 2024

View reviewed changes

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch from 984a410 to 057bee2 Compare December 26, 2024 15:07

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch 2 times, most recently from 9e9eea0 to 7dd73e0 Compare December 29, 2024 13:28

noaov1 requested changes Dec 29, 2024

View reviewed changes

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch from 7dd73e0 to 00a9ce2 Compare December 29, 2024 14:58

noaov1 approved these changes Dec 29, 2024

View reviewed changes

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch 2 times, most recently from 6f66dfd to 9d03378 Compare January 2, 2025 08:43

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch 3 times, most recently from 34237fd to e3700e9 Compare January 2, 2025 12:49

noaov1 approved these changes Jan 2, 2025

View reviewed changes

meship-starkware reviewed Jan 2, 2025

View reviewed changes

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch from e3700e9 to a47b870 Compare January 2, 2025 16:37

meship-starkware reviewed Jan 5, 2025

View reviewed changes

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch 3 times, most recently from 27b48b4 to f9c5cc2 Compare January 5, 2025 15:23

meship-starkware approved these changes Jan 5, 2025

View reviewed changes

noaov1 approved these changes Jan 5, 2025

View reviewed changes

chore(blockifier): remove syscall_gas_costs from os_constans

edc0aa0

Yonatan-Starkware force-pushed the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch from f9c5cc2 to edc0aa0 Compare January 5, 2025 17:37

noaov1 approved these changes Jan 5, 2025

View reviewed changes

noaov1 approved these changes Jan 6, 2025

View reviewed changes

noaov1 merged commit 92bc603 into main-v0.13.4 Jan 6, 2025
11 checks passed

noaov1 deleted the yonatank/blockifier/remove_syscall_gas_costs_from_os_constants branch January 6, 2025 06:24

github-actions bot locked and limited conversation to collaborators Jan 7, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(blockifier): remove syscall_gas_costs from os_constans #2864

chore(blockifier): remove syscall_gas_costs from os_constans #2864

Yonatan-Starkware commented Dec 22, 2024

reviewable-StarkWare commented Dec 22, 2024

github-actions bot commented Dec 22, 2024 •

edited

Loading

Yonatan-Starkware commented Dec 22, 2024 •

edited

Loading

meship-starkware left a comment

meship-starkware left a comment

meship-starkware left a comment

noaov1 left a comment

Yonatan-Starkware commented Dec 26, 2024

Yonatan-Starkware left a comment

noaov1 left a comment

Yonatan-Starkware commented Dec 29, 2024

noaov1 left a comment

Yonatan-Starkware commented Jan 2, 2025

noaov1 left a comment

meship-starkware left a comment

Yonatan-Starkware commented Jan 2, 2025

meship-starkware left a comment

meship-starkware left a comment

noaov1 left a comment

Yonatan-Starkware commented Jan 5, 2025

noaov1 left a comment

noaov1 left a comment

chore(blockifier): remove syscall_gas_costs from os_constans #2864

chore(blockifier): remove syscall_gas_costs from os_constans #2864

Conversation

Yonatan-Starkware commented Dec 22, 2024

reviewable-StarkWare commented Dec 22, 2024

github-actions bot commented Dec 22, 2024 • edited Loading

Yonatan-Starkware commented Dec 22, 2024 • edited Loading

meship-starkware left a comment

Choose a reason for hiding this comment

meship-starkware left a comment

Choose a reason for hiding this comment

meship-starkware left a comment

Choose a reason for hiding this comment

noaov1 left a comment

Choose a reason for hiding this comment

Yonatan-Starkware commented Dec 26, 2024

Yonatan-Starkware left a comment

Choose a reason for hiding this comment

noaov1 left a comment

Choose a reason for hiding this comment

Yonatan-Starkware commented Dec 29, 2024

noaov1 left a comment

Choose a reason for hiding this comment

Yonatan-Starkware commented Jan 2, 2025

noaov1 left a comment

Choose a reason for hiding this comment

meship-starkware left a comment

Choose a reason for hiding this comment

Yonatan-Starkware commented Jan 2, 2025

meship-starkware left a comment

Choose a reason for hiding this comment

meship-starkware left a comment

Choose a reason for hiding this comment

noaov1 left a comment

Choose a reason for hiding this comment

Yonatan-Starkware commented Jan 5, 2025

noaov1 left a comment

Choose a reason for hiding this comment

noaov1 left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 22, 2024 •

edited

Loading

Yonatan-Starkware commented Dec 22, 2024 •

edited

Loading