Bump blockifier to v0.13.5 #3130

ddoktorski · 2025-03-21T13:25:02Z

Stack:

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

ddoktorski · 2025-03-21T13:32:09Z

Towards #3037

Note: It is not possible to use blockifier from the Starkware repository at the moment due to a conflict with the cairo-lang-sierra dependency. It is strictly set to =2.10.0 there, whereas we have already bumped cairo dependencies to 2.11.2. A necessary modification has been made in my forked repository, which is being used for now.

THenry14

all the stuff is rather nitpicky so it's a +1 from me, but please address all the comments first (one way or the other). Great job 💪

...heatnet/src/runtime_extensions/call_to_blockifier_runtime_extension/execution/entry_point.rs

crates/cheatnet/src/runtime_extensions/deprecated_cheatable_starknet_extension/mod.rs

THenry14 · 2025-03-27T09:57:56Z

crates/cheatnet/src/runtime_extensions/forge_runtime_extension/mod.rs

-                    versioned_constants.get_syscall_gas_cost(name) * (*count as u64)
+                .map(|(selector, usage)| {
+                    let syscall_gas_cost = versioned_constants.get_syscall_gas_cost(selector);
+                    let base_cost = syscall_gas_cost.get_syscall_cost(0);


as per comment above. idea: maybe we could make this logic common, put it in a function or something?

Added get_syscalls_gas_consumed function

Cargo.toml

crates/forge-runner/src/printing.rs

crates/forge/tests/integration/resources.rs

crates/forge-runner/src/printing.rs

cptartur · 2025-03-28T09:47:34Z

...et/src/runtime_extensions/call_to_blockifier_runtime_extension/execution/cheated_syscalls.rs

+    // Increment the Deploy syscall's linear cost counter by the number of elements in the
+    // constructor calldata
+    syscall_handler.increment_linear_factor_by(
+        &SyscallSelector::Deploy,
+        request.constructor_calldata.0.len(),
+    );
+


Does this affect the gas tests? Or is the linear factor too small to contribute to them?

It does affect gas costs, which is reflected in the sierra gas tests below. When tracking Cairo steps it is negligible in our tests due to different fee formulat. We could potentially add a test that has it reflected for Cairo steps, but it would require calling deploy with calldata with at least 4 or 5 elements to see any change.

Maybe it'd be worth adding a test like this. If we already are trying to be quite throughout with our gas tests.

cptartur · 2025-03-28T09:48:49Z

...heatnet/src/runtime_extensions/call_to_blockifier_runtime_extension/execution/entry_point.rs

+    let entry_point = ExecutableCallEntryPoint {
+        class_hash,
+        code_address: entry_point.code_address,
+        entry_point_type: entry_point.entry_point_type,
+        entry_point_selector: entry_point.entry_point_selector,
+        calldata: entry_point.calldata.clone(),
+        storage_address: entry_point.storage_address,
+        caller_address: entry_point.caller_address,
+        call_type: entry_point.call_type,
+        initial_gas: entry_point.initial_gas,
+    };


Why do we need to reconstruct the entire entrypoint now? Not a big problem but I just wonder.

I was wondering as well, but it seems these are actually different structs now (ExecutableCallEntryPoint vs CallEntryPoint), and I don't think there is any way in blockifier to cast one to another

We need to pass ExecutableCallEntryPoint not CallEntryPoint to execute_entry_point_call_cairo1 below. There is a function into_executable but it is private.

Let's create a PR that makes it public and an issue so we can update this eventually.

...heatnet/src/runtime_extensions/call_to_blockifier_runtime_extension/execution/entry_point.rs

crates/cheatnet/src/runtime_extensions/cheatable_starknet_runtime_extension.rs

crates/cheatnet/tests/common/mod.rs

crates/forge-runner/src/printing.rs

crates/forge/tests/integration/gas.rs

commit-id:1bd782e3

cptartur

See my outstanding comments, otherwise looks good

cptartur · 2025-03-28T15:50:43Z

crates/cheatnet/src/runtime_extensions/common.rs

+            // We want to calculate `base_cost * call_count + linear_cost * linear_factor`
+            // There is a field name `linear_factor` used in both `SyscallUsage` and `SyscallGasCost`
+            // In `get_syscall_cost` function `linear_length` parameter is calldata length, hence `SyscallUsage.linear_factor`


What I meant is that the math to get these values is not 100% obvious (i.e. reason for call_count - 1).

But the extra context is also useful.

ddoktorski requested a review from a team as a code owner March 21, 2025 13:25

ddoktorski requested review from franciszekjob and kkawula March 21, 2025 13:25

This was referenced Mar 21, 2025

Use ArchivalDataResources from blockifier #3132

Open

Remove copied code from cairo1_execution.rs #3131

Open

ddoktorski marked this pull request as draft March 21, 2025 13:25

ddoktorski requested a review from cptartur March 21, 2025 13:26

ddoktorski force-pushed the spr/master/1bd782e3 branch from dce17db to bbb2a19 Compare March 26, 2025 12:59

ddoktorski marked this pull request as ready for review March 26, 2025 13:58

ddoktorski requested a review from THenry14 March 26, 2025 14:54

THenry14 approved these changes Mar 27, 2025

View reviewed changes

franciszekjob reviewed Mar 27, 2025

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

crates/forge-runner/src/printing.rs Outdated Show resolved Hide resolved

crates/forge/tests/integration/resources.rs Show resolved Hide resolved

franciszekjob reviewed Mar 28, 2025

View reviewed changes

crates/forge-runner/src/printing.rs Outdated Show resolved Hide resolved

cptartur reviewed Mar 28, 2025

View reviewed changes

ddoktorski force-pushed the spr/master/1bd782e3 branch from bbb2a19 to 9ddd29f Compare March 28, 2025 12:36

Bump blockifier to v0.13.5

74d2f3a

commit-id:1bd782e3

ddoktorski force-pushed the spr/master/1bd782e3 branch from 9ddd29f to 74d2f3a Compare March 28, 2025 13:05

franciszekjob approved these changes Mar 28, 2025

View reviewed changes

ddoktorski requested a review from cptartur March 28, 2025 15:35

ddoktorski mentioned this pull request Mar 28, 2025

Investigate gas refund logic #3155

Open

cptartur approved these changes Mar 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump blockifier to v0.13.5 #3130

Bump blockifier to v0.13.5 #3130

ddoktorski commented Mar 21, 2025 •

edited

Loading

ddoktorski commented Mar 21, 2025

THenry14 left a comment

THenry14 Mar 27, 2025

ddoktorski Mar 28, 2025

cptartur Mar 28, 2025

ddoktorski Mar 28, 2025

cptartur Mar 28, 2025

cptartur Mar 28, 2025

THenry14 Mar 28, 2025

ddoktorski Mar 28, 2025

cptartur Mar 28, 2025

cptartur left a comment

cptartur Mar 28, 2025

Bump blockifier to v0.13.5 #3130

Are you sure you want to change the base?

Bump blockifier to v0.13.5 #3130

Conversation

ddoktorski commented Mar 21, 2025 • edited Loading

ddoktorski commented Mar 21, 2025

THenry14 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cptartur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddoktorski commented Mar 21, 2025 •

edited

Loading