feat(blockifier): refactor native stack config

[AvivYossef-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2024-12-29T15:04:47Z
• Started at 2024-12-30T08:19:32Z
• Started at 2024-12-30T12:24:33Z
• Started at 2024-12-30T13:26:26Z
• Started at 2024-12-30T15:26:54Z
• Started at 2025-01-01T07:04:04Z
• Started at 2025-01-01T07:05:06Z
• Started at 2025-01-01T08:28:37Z
• Started at 2025-01-01T09:12:09Z
• Started at 2025-01-01T09:33:02Z

[AvivYossef-starkware]

• chore(blockifier): add cairo native stack config to override vc #3022
• chore(blockifier): add nstive stack config to vc #3008
• feat(blockifier): refactor native stack config #3003 👈 (View in Graphite)
• main-v0.13.4

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

[Yoni-Starkware]

Reviewed 1 of 3 files at r1, all commit messages.
Reviewable status: 1 of 3 files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/blockifier/src/versioned_constants.rs line 503 at r1 (raw file):

    pub fn get_target_stack_size(&self, red_zone: usize) -> usize {
        std::cmp::min(red_zone + self.buffer_size, self.max_stack_size)
    }

Take the min in get_stack_size_red_zone as well, not here.
Add the self.max_stack_size to the red zone without capping to get the target size.

---

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

        max_stack_size: 170 * 1024 * 1024,
        min_stack_red_zone: 2 * 1024 * 1024,
        buffer_size: 1024 * 1024,

To be on the safe side

Suggestion:

        gas_to_stack_ratio: Ratio::new(1, 20),
        max_stack_size: 200 * 1024 * 1024,
        min_stack_red_zone: 2 * 1024 * 1024,
        buffer_size: 5 * 1024 * 1024,

---

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

    let stack_size_red_zone = stack_config.get_stack_size_red_zone(
        usize::try_from(syscall_handler.base.call.initial_gas)
            .expect("initial gas to usize should always succeed"),

Not sure about it. usize can be 32 bit.
The stack size should be usize, try to convert at the end.

Code quote:

.expect("initial gas to usize should always succeed"),

[Yoni-Starkware]

Reviewable status: 1 of 3 files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/blockifier/src/versioned_constants.rs line 503 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Take the min in get_stack_size_red_zone as well, not here.
Add the self.max_stack_size to the red zone without capping to get the target size.

(I want the target size to always be greater then the red zone)

[AvivYossef-starkware]

Reviewable status: 1 of 3 files reviewed, 3 unresolved discussions (waiting on @Yoni-Starkware)

---

crates/blockifier/src/versioned_constants.rs line 503 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

(I want the target size to always be greater then the red zone)

So, don't we want to limit the stack size?
In that case, do we need both buffer_size and max_stack_size?

---

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

Previously, Yoni-Starkware (Yoni) wrote…

To be on the safe side

Done.

---

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

Previously, Yoni-Starkware (Yoni) wrote…

Not sure about it. usize can be 32 bit.
The stack size should be usize, try to convert at the end.

Done

[Yoni-Starkware]

Reviewable status: 1 of 3 files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/blockifier/src/execution/native/entry_point_execution.rs line 71 at r2 (raw file):

    let target_stack_size =
        usize::try_from(stack_config.get_target_stack_size(stack_size_red_zone))
            .map_err(|e| EntryPointExecutionError::InternalError(e.to_string()))?;

You should panic here - it is our problem, not the user's (returning an error will fail the transaction)

Code quote:

.map_err(|e| EntryPointExecutionError::InternalError(e.to_string()))?;

[Yoni-Starkware]

Reviewable status: 1 of 3 files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/blockifier/src/versioned_constants.rs line 503 at r1 (raw file):

Previously, AvivYossef-starkware wrote…

So, don't we want to limit the stack size?
In that case, do we need both buffer_size and max_stack_size?

We want:

• to limit the stack size
• to have the target size always red_zone + buffer.

So the "real" max is the config + buffer, which is fine.

[AvivYossef-starkware]

Reviewable status: 1 of 3 files reviewed, 3 unresolved discussions (waiting on @Yoni-Starkware)

---

crates/blockifier/src/versioned_constants.rs line 503 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

We want:

• to limit the stack size
• to have the target size always red_zone + buffer.

So the "real" max is the config + buffer, which is fine.

Ok so I'll limit the min < red zone < max
and always add the buffer size

---

crates/blockifier/src/execution/native/entry_point_execution.rs line 71 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

You should panic here - it is our problem, not the user's (returning an error will fail the transaction)

Done.

[Yoni-Starkware]

Reviewed all commit messages.
Reviewable status: 1 of 3 files reviewed, 4 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/blockifier/src/versioned_constants.rs line 498 at r3 (raw file):

        std::cmp::max(
            self.max_stack_size,
            std::cmp::min(

Oops.
Use clamp instead (google it)

Suggestion:

        std::cmp::min(
            self.max_stack_size,
            std::cmp::max(

[Yoni-Starkware]

Reviewable status: 1 of 3 files reviewed, 5 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/blockifier/src/versioned_constants.rs line 494 at r3 (raw file):

impl CairoNativeStackConfig {
    /// Returns the size of the stack red zone, relative to the remaining gas.

Suggestion:

    /// Returnד the stack size sufficient for running Cairo Native with the given gas.

[AvivYossef-starkware]

Reviewable status: 1 of 3 files reviewed, 4 unresolved discussions (waiting on @Yoni-Starkware)

---

crates/blockifier/src/versioned_constants.rs line 498 at r3 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Oops.
Use clamp instead (google it)

Done.

---

crates/blockifier/src/versioned_constants.rs line 494 at r3 (raw file):

impl CairoNativeStackConfig {
    /// Returns the size of the stack red zone, relative to the remaining gas.

Done.

[Yoni-Starkware]

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

---

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

    let execution_result = stacker::maybe_grow(
        usize::try_from(stack_size_red_zone)
            .unwrap_or_else(|e| panic!("Failed to convert stack size red zone to usize: {}", e)),

Move the convesion up, to be consistent with target_stack_size

Code quote:

        usize::try_from(stack_size_red_zone)
            .unwrap_or_else(|e| panic!("Failed to convert stack size red zone to usize: {}", e)),

[AvivYossef-starkware]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @Yoni-Starkware)

---

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

Previously, Yoni-Starkware (Yoni) wrote…

Move the convesion up, to be consistent with target_stack_size

Done.

[Yoni-Starkware]

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

---

crates/blockifier/src/versioned_constants.rs line 512 at r5 (raw file):

    pub fn get_target_stack_size(&self, red_zone: u64) -> u64 {
        red_zone + self.buffer_size

This is the size that matters. Round also here to be on the safe side.

Code quote:

red_zone + self.buffer_size

[Yoni-Starkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/blockifier/src/versioned_constants.rs line 511 at r5 (raw file):

    }

    pub fn get_target_stack_size(&self, red_zone: u64) -> u64 {

Also, document that the stack size must be in page_size multiplications since there is a bug in stacker::grow

Code quote:

    pub fn get_target_stack_size(&self, red_zone: u64) -> u64 {

[AvivYossef-starkware]

Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on @Yoni-Starkware)

---

crates/blockifier/src/versioned_constants.rs line 511 at r5 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Also, document that the stack size must be in page_size multiplications since there is a bug in stacker::grow

Ok, I wasn't sure if we want to make it public

---

crates/blockifier/src/versioned_constants.rs line 512 at r5 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

This is the size that matters. Round also here to be on the safe side.

Done.

[Yoni-Starkware]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/blockifier/src/versioned_constants.rs line 511 at r5 (raw file):

Previously, AvivYossef-starkware wrote…

Ok, I wasn't sure if we want to make it public

Hmm okay, so let's rephrase

        // Stack size should be a multiple of page size, since `stacker::grow` works with this unit.

---

crates/blockifier/src/versioned_constants.rs line 497 at r6 (raw file):

    pub fn round_up_to_mb(size: u64) -> u64 {
        const MB: u64 = 1024 * 1024;
        if size % MB == 0 { size } else { ((size / MB) + 1) * MB }

Consider

Suggestion:

((size + MB - 1) / MB) * MB

---

crates/blockifier/src/versioned_constants.rs line 513 at r6 (raw file):

    pub fn get_target_stack_size(&self, red_zone: u64) -> u64 {
        // Stack size must be a multiple of page size.
        // Since there is a bug in `stacker::grow`.

[AvivYossef-starkware]

Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @Yoni-Starkware)

---

crates/blockifier/src/versioned_constants.rs line 497 at r6 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Consider

The compiler asked me to use div_ceil ( its the same )

[Yoni-Starkware]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @AvivYossef-starkware)

---

crates/blockifier/src/versioned_constants.rs line 497 at r6 (raw file):

Previously, AvivYossef-starkware wrote…

The compiler asked me to use div_ceil ( its the same )

Don't you need to multiply by MB?

[Yoni-Starkware]

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