Update blockifier #2397

AnkushinDaniil · 2025-01-24T19:00:37Z

This pull request includes several changes to the vm module, primarily focusing on extending the class information in vm/class.go, updating dependencies in Cargo.toml.

Class Information Extension:

Updated the marshalClassInfo function in vm/class.go to include ClassVersion and SierraVersion fields for both Cairo0Class and CompiledClass. [1] [2]

Dependency Updates:

Updated dependencies in vm/rust/Cargo.toml to newer versions, including blockifier, cairo-lang-starknet-classes, cairo-lang-runner, starknet_api, and starknet-types-core.

codecov · 2025-01-24T19:09:04Z

Codecov Report

Attention: Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.

Project coverage is 74.40%. Comparing base (1caf86f) to head (8617b5c).

Files with missing lines	Patch %	Lines
rpc/trace.go	44.44%	4 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2397      +/-   ##
==========================================
- Coverage   74.41%   74.40%   -0.02%     
==========================================
  Files         113      113              
  Lines       13016    13030      +14     
==========================================
+ Hits         9686     9695       +9     
- Misses       2592     2597       +5     
  Partials      738      738

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

vm/rust/src/juno_state_reader.rs

vm/rust/src/lib.rs

rianhughes · 2025-01-29T15:08:21Z

vm/rust/src/lib.rs

+    // TODO: initial gas should be different based on the execution method used: vm or native!
+    // There should be some check.
+    let initial_gas = get_versioned_constants(block_info.version).infinite_gas_for_vm_mode();
+


Note for myself to come back to this

I think the todo text can be misleading. We shouldn't speak in terms of vm or native, but in terms of vm and sierra gas. The latter is independent of the execution engine (native or vm) while the former gas functionality is bounded to the vm functionality

It seems that the initial gas will depend on the transaction fields in the future, but this looks correct for now

// An estimation of the initial gas for a transaction to run with. This solution is // temporary and this value will be deduced from the transaction's fields. pub default_initial_gas_cost: u64,

I think this initial_gas is assuming that we are going by default to execute using the previous gas scheme. There should be a check here that asks which contract class we are executing:

Are we charging "vm gas" or are we charging "sierra gas"?

Sierra gas only applies for >2.9.0 Sierra version in the contracts. VM gas for everything else.

I think there are other exceptions such as that if a contract begins executing itself with VM gas, then all the other calls to external contracts should stick to this gas mechanism (even if it fits to use Sierra gas). I need to read the release notes more thoroughly, because I don't know if it is exactly these or something slightly different.

rodrigo-pino

First round of review. Will do another pass later. Overall, everything looks good to me.

rodrigo-pino · 2025-01-30T10:05:33Z

vm/rust/resources/versioned_constants_0_13_0.json

I've seen that the constants version files 0_13_0, 0_13_1 and 0_13_1_1 have changed. Are these changes consistent with the sequencer repo?

rodrigo-pino · 2025-01-30T10:07:12Z

vm/rust/resources/versioned_constants_0_13_2.json

Also, there are new additions such as 0_13_2, 0_13_2_1, 0_13_3 and 0_13_4. We didn't have any files before and Juno was working.

Does the addition of these new files change anything? The answer should be yes, but I wonder why Juno was working correctly when it didn't have this files. Maybe we are missing checks/tests?

Yes, we are. We only cover empty data cases.

vm/rust/src/juno_state_reader.rs

rodrigo-pino · 2025-01-30T10:58:50Z

vm/rust/src/lib.rs

+    // TODO: initial gas should be different based on the execution method used: vm or native!
+    // There should be some check.
+    let initial_gas = get_versioned_constants(block_info.version).infinite_gas_for_vm_mode();
+


I think this initial_gas is assuming that we are going by default to execute using the previous gas scheme. There should be a check here that asks which contract class we are executing:

Are we charging "vm gas" or are we charging "sierra gas"?

Sierra gas only applies for >2.9.0 Sierra version in the contracts. VM gas for everything else.

I think there are other exceptions such as that if a contract begins executing itself with VM gas, then all the other calls to external contracts should stick to this gas mechanism (even if it fits to use Sierra gas). I need to read the release notes more thoroughly, because I don't know if it is exactly these or something slightly different.

vm/vm_bridge.h

core/transaction.go

rianhughes · 2025-01-31T14:39:22Z

vm/rust/src/lib.rs

+                let da_gas_l1_gas = t.receipt.da_gas.l1_gas.into();
+                let da_gas_l1_data_gas = t.receipt.da_gas.l1_data_gas.into();
                let execution_steps = t


Do we also need to return l2_gas in JunoAppendGasConsumed?

rianhughes · 2025-01-31T14:42:51Z

Up to the above comments the PR ltgm

AnkushinDaniil added VM dependencies Pull requests that update a dependency file go Pull requests that update Go code rust Pull requests that update Rust code labels Jan 24, 2025

AnkushinDaniil requested a review from rodrigo-pino January 24, 2025 19:00

AnkushinDaniil requested a review from cicr99 January 24, 2025 19:23

rianhughes mentioned this pull request Jan 29, 2025

implement logic to build genesis state for the sequencer #2371

Open