Update blockifier #2397
Update blockifier #2397
Conversation
AnkushinDaniil
added
VM
dependencies
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2397 +/- ##
==========================================
- Coverage 74.48% 74.43% -0.06%
==========================================
Files 113 113
Lines 13020 13020
==========================================
- Hits 9698 9691 -7
- Misses 2587 2592 +5
- Partials 735 737 +2 ☔ View full report in Codecov by Sentry. |
| @@ -131,7 +131,7 @@ impl StateReader for JunoStateReader { | |||
| } | |||
|
|
|||
| /// Returns the contract class of the given class hash. | |||
| fn get_compiled_contract_class(&self, class_hash: ClassHash) -> StateResult<ContractClass> { | |||
| fn get_compiled_class(&self, class_hash: ClassHash) -> StateResult<RunnableCompiledClass> { | |||
There was a problem hiding this comment.
It's not possible for this method to return a V1Native(NativeCompiledClassV1) right? I'm just wondering if we should distinguish this case separately
| 0 => { | ||
| sierra_version = SierraVersion::DEPRECATED; | ||
| match parse_deprecated_class_definition(class_def.to_string()) { | ||
| Ok(class) => class, | ||
| Err(err) => return Err(format!("failed parsing deprecated class: {:?}", err)), | ||
| } | ||
| } |
There was a problem hiding this comment.
Are abi_length and sierra_len still set to zero? I believe we need to manually override these fields for Cairo0 contracts, because abi_len affects the fee (and therefore traces and receipts), and originally these fields were ignored when generating the fee.
| } | ||
|
|
||
| // TODO: Should I add versioned constants 13.4?. Where are 13.3? |
| // the genesis block has a gas price of 0, but the blockifier | ||
| // does not allow for this. This, however, is not critical | ||
| // for consensus, and so the value of 1 is used instead. | ||
| fn gas_price_from_bytes(bytes: &[c_uchar; 32]) -> Result<NonzeroGasPrice, anyhow::Error> { |
There was a problem hiding this comment.
What do you think about renaming this as gas_price_from_bytes_min1 or gas_price_from_bytes_bonded or something?
There was a problem hiding this comment.
Also, what do you mean that this isn't ciritical for consensus?
| let actual_fee: Felt = t.receipt.fee.0.into(); | ||
| 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(); | ||
| // TODO: Are these resurce usage still valid? |
| // TODO: what goes here? Random value currently | ||
| l2_gas: GasAmount(8), |
| // TODO: Which si the right gas vector, how to select it | ||
| &GasVectorComputationMode::All, |
There was a problem hiding this comment.
Might be worth asking the SN guys about this
| // 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(); | ||
|
|
There was a problem hiding this comment.
Note for myself to come back to this
This pull request includes several changes to the
vmmodule, primarily focusing on extending the class information invm/class.go, updating dependencies inCargo.toml.Class Information Extension:
marshalClassInfofunction invm/class.goto includeClassVersionandSierraVersionfields for bothCairo0ClassandCompiledClass. [1] [2]Dependency Updates:
vm/rust/Cargo.tomlto newer versions, includingblockifier,cairo-lang-starknet-classes,cairo-lang-runner,starknet_api, andstarknet-types-core.