build(blockifier): add get class hash at syscall

[AvivYossef-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[AvivYossef-starkware]

• build(blockifier): add get class hash at syscall #1937 👈
• main

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

Join @AvivYossef-starkware and the rest of your teammates on Graphite

[github-actions]

Artifacts upload triggered. View details here

[AvivYossef-starkware]

Should wait before merge, it probably brakes python

[codecov]

Codecov Report

Attention: Patch coverage is 96.20253% with 3 lines in your changes missing coverage. Please review.

Project coverage is 69.15%. Comparing base (e3165c4) to head (ae5eb96).
Report is 388 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/blockifier/src/execution/syscalls/mod.rs	84.21%	0 Missing and 3 partials ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1937       +/-   ##
===========================================
+ Coverage   40.10%   69.15%   +29.05%     
===========================================
  Files          26      104       +78     
  Lines        1895    13641    +11746     
  Branches     1895    13641    +11746     
===========================================
+ Hits          760     9434     +8674     
- Misses       1100     3804     +2704     
- Partials       35      403      +368     

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

[AvivYossef-starkware]

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

---

crates/blockifier/src/versioned_constants.rs line 524 at r2 (raw file):

        D: Deserializer<'de>,
    {
        let mut os_resources = Self::deserialize(deserializer)?;

Should I add it to the JSON file or deserialize it like that?

[github-actions]

Artifacts upload triggered. View details here

[AvivYossef-starkware]

python
https://github.com/starkware-industries/starkware/pull/36215

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

[github-actions]

Artifacts upload triggered. View details here

[Yoni-Starkware]

Reviewed 4 of 12 files at r1, 1 of 1 files at r3.
Reviewable status: 5 of 13 files reviewed, 12 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/blockifier/src/versioned_constants.rs line 524 at r2 (raw file):

Previously, AvivYossef-starkware wrote…

Should I add it to the JSON file or deserialize it like that?

Add to the json with price 0 (consistent with other new syscalls such as sha256_process_block_gas_cost)

---

crates/blockifier/resources/versioned_constants_0_13_4.json line 648 at r3 (raw file):

        ]
    }
}

revert

---

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

    StorageRead,
    StorageWrite,
    GetClassHashAt,

Sort

Code quote:

    GetClassHashAt,

---

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

            b"StorageRead" => Ok(Self::StorageRead),
            b"StorageWrite" => Ok(Self::StorageWrite),
            b"GetClassHashAt" => Ok(Self::GetClassHashAt),

Sort

Code quote:

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

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 234 at r3 (raw file):

    pub read_class_hash_values: Vec<ClassHash>,
    pub accessed_contract_addresses: HashSet<ContractAddress>,

To make the use case clear (since there are other accessed addresses that are not in this list, such as internal call contracts)

Suggestion:

    // Additional information gathered during execution.
    pub read_values: Vec<Felt>,
    pub accessed_keys: HashSet<StorageKey>,
    pub read_class_hash_values: Vec<ClassHash>,
    // Accessed addresses by the `get_class_hash_at` syscall.
    pub accessed_contract_addresses: HashSet<ContractAddress>,

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 441 at r3 (raw file):

                self.context.gas_costs().storage_write_gas_cost,
            ),
            SyscallSelector::GetClassHashAt => self.execute_syscall(

Sort

Code quote:

SyscallSelector::GetClassHashAt => self.execute_syscall(

---

crates/blockifier/src/execution/deprecated_entry_point_execution.rs line 289 at r3 (raw file):

        accessed_storage_keys: syscall_handler.accessed_keys,
        accessed_contract_addresses: syscall_handler.accessed_contract_addresses,
        read_class_hash_values: syscall_handler.read_class_hash_values,

Revert. We don't support this syscall for cairo 0.

Code quote:

        accessed_contract_addresses: syscall_handler.accessed_contract_addresses,
        read_class_hash_values: syscall_handler.read_class_hash_values,

---

crates/blockifier/src/execution/deprecated_syscalls/hint_processor.rs line 164 at r3 (raw file):

/// Executes Starknet syscalls (stateful protocol hints) during the execution of an entry point
/// call.
pub struct DeprecatedSyscallHintProcessor<'a> {

Revert. We don't support this syscall for cairo 0.

---

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

        accessed_storage_keys: syscall_handler.accessed_keys,
        accessed_contract_addresses: HashSet::new(),
        read_class_hash_values: Vec::new(),

Add a TODO instead. The syscall is not supported here yet

Suggestion:

        accessed_storage_keys: syscall_handler.accessed_keys,

[Yoni-Starkware]

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

---

crates/blockifier/src/execution/syscalls/syscall_tests/get_class_hash_at.rs line 35 at r3 (raw file):

    let positive_call_info = positive_entry_point_call.execute_directly(&mut state).unwrap();
    assert!(positive_call_info.accessed_contract_addresses.contains(&address));
    assert!(positive_call_info.read_class_hash_values.contains(&class_hash));

Suggestion:

positive_call_info.read_class_hash_values[0] == &class_hash

[github-actions]

Artifacts upload triggered. View details here

[AvivYossef-starkware]

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

---

crates/blockifier/resources/versioned_constants_0_13_4.json line 648 at r3 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

revert

Done.

---

crates/blockifier/src/versioned_constants.rs line 524 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Add to the json with price 0 (consistent with other new syscalls such as sha256_process_block_gas_cost)

Done.

---

crates/blockifier/src/execution/deprecated_entry_point_execution.rs line 289 at r3 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Revert. We don't support this syscall for cairo 0.

its a field of call info so I should put there somthing

---

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

Previously, Yoni-Starkware (Yoni) wrote…

Sort

Done.

---

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

Previously, Yoni-Starkware (Yoni) wrote…

Sort

Done.

---

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

Previously, Yoni-Starkware (Yoni) wrote…

Add a TODO instead. The syscall is not supported here yet

Ok, but I was needed to add #[allow(unreachable\_code)]
to make it compile
cargo build -p blockifier --features cairo_native

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 234 at r3 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

To make the use case clear (since there are other accessed addresses that are not in this list, such as internal call contracts)

Done.

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 441 at r3 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Sort

Done.

---

crates/blockifier/src/execution/deprecated_syscalls/hint_processor.rs line 164 at r3 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Revert. We don't support this syscall for cairo 0.

Done.

[AvivYossef-starkware]

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

---

crates/blockifier/src/execution/syscalls/syscall_tests/get_class_hash_at.rs line 35 at r3 (raw file):

    let positive_call_info = positive_entry_point_call.execute_directly(&mut state).unwrap();
    assert!(positive_call_info.accessed_contract_addresses.contains(&address));
    assert!(positive_call_info.read_class_hash_values.contains(&class_hash));

Done.

[github-actions]

Artifacts upload triggered. View details here

[Yoni-Starkware]

Reviewed 2 of 12 files at r1, 10 of 15 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: 15 of 19 files reviewed, 6 unresolved discussions (waiting on @AvivYossef-starkware)

---

crates/blockifier/src/execution/deprecated_entry_point_execution.rs line 289 at r3 (raw file):

Previously, AvivYossef-starkware wrote…

its a field of call info so I should put there somthing

Default is great

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 441 at r3 (raw file):

Previously, AvivYossef-starkware wrote…

Done.

Not yet :)

---

crates/blockifier/src/execution/syscalls/syscall_tests/get_class_hash_at.rs line 28 at r5 (raw file):

    state.state.address_to_class_hash.insert(address, class_hash);

    // Positive case: address and class_hash are correct

Suggestion:

    // Test deployed contract.

---

crates/blockifier/src/execution/syscalls/syscall_tests/get_class_hash_at.rs line 47 at r5 (raw file):

    );

    // Negative case 1: Non-existing address should return class_hash = 0 and succeed.

Suggestion:

// Test undeployed contract - should return class_hash = 0 and succeed.

---

crates/blockifier/src/execution/syscalls/syscall_tests/get_class_hash_at.rs line 52 at r5 (raw file):

    let negative_entry_point_call = CallEntryPoint {
        calldata: calldata![non_existing_address.into(), class_hash_of_undeployed_contract.0],

Suggestion:

    let non_existing_address = felt!("0x333");
    let class_hash_of_undeployed_contract = felt!("0x0");

    let negative_entry_point_call = CallEntryPoint {
        calldata: calldata![non_existing_address, class_hash_of_undeployed_contract],

---

crates/blockifier/src/execution/syscalls/syscall_tests/get_class_hash_at.rs line 58 at r5 (raw file):

    assert!(!negative_entry_point_call.execute_directly(&mut state).unwrap().execution.failed);

    // Negative case 2: Existing address but mismatched class_hash should fail.

You're testing the test here, it's not a negative case of the user.

Suggestion:

    // Sanity check: giving the wrong expected class hash to the test should make it fail.

---

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

        // TODO(Aviv): The syscall is not supported here yet.
        // Currently, `accessed_contract_addresses` and `read_class_hash_values` are initialized
        // as empty. Support for handling accessed storage keys via syscalls should be implemente

Suggestion:

ld be implemented.

[github-actions]

Artifacts upload triggered. View details here

[AvivYossef-starkware]

Reviewable status: 13 of 19 files reviewed, 5 unresolved discussions (waiting on @Yoni-Starkware)

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 441 at r3 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Not yet :)

ops

---

crates/blockifier/src/execution/syscalls/syscall_tests/get_class_hash_at.rs line 58 at r5 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

You're testing the test here, it's not a negative case of the user.

Done.

---

crates/blockifier/src/execution/syscalls/syscall_tests/get_class_hash_at.rs line 28 at r5 (raw file):

    state.state.address_to_class_hash.insert(address, class_hash);

    // Positive case: address and class_hash are correct

Done.

---

crates/blockifier/src/execution/syscalls/syscall_tests/get_class_hash_at.rs line 47 at r5 (raw file):

    );

    // Negative case 1: Non-existing address should return class_hash = 0 and succeed.

Done.

---

crates/blockifier/src/execution/syscalls/syscall_tests/get_class_hash_at.rs line 52 at r5 (raw file):

    let negative_entry_point_call = CallEntryPoint {
        calldata: calldata![non_existing_address.into(), class_hash_of_undeployed_contract.0],

Done.

[Yoni-Starkware]

BTW, why build?

(Wait for python)

Reviewed 1 of 15 files at r4, 4 of 4 files at r6, all commit messages.
Reviewable status: 18 of 19 files reviewed, all discussions resolved

[github-actions]

Artifacts upload triggered. View details here

[AvivYossef-starkware]

I named it build because the commit adds a new feature. What would you call it?

Reviewable status: 17 of 19 files reviewed, all discussions resolved (waiting on @Yoni-Starkware)

[Yoni-Starkware]

feat? but it's not important

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: 18 of 19 files reviewed, all discussions resolved

[AvivYossef-starkware]

Ok, next time.
Can I merge? Python passed all checks

Reviewable status: 18 of 19 files reviewed, all discussions resolved (waiting on @Yoni-Starkware)

[Yoni-Starkware]

Yes

Reviewable status: 18 of 19 files reviewed, all discussions resolved

[Yoni-Starkware]

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

[AvivYossef-starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @AvivYossef-starkware)

[AvivYossef-starkware]

Merge activity

• Nov 14, 6:26 AM EST: A user merged this pull request with Graphite.