refactor: split syscalls into separate files

[varex83]

This PR is part of the series of PRs that brings cairo-native to the blockifier. These changes are need because they create more generic testing approach that could be applied for NativeSyscallHandler.

---

This change is 

[Yoni-Starkware]

Reviewed 17 of 19 files at r1.
Reviewable status: 14 of 19 files reviewed, 9 unresolved discussions (waiting on @varex83)

---

crates/blockifier/src/execution/syscalls/syscall_tests/call_contract.rs line 57 at r1 (raw file):

    );

    // ensure that the fallback system didn't replace the contract

Comment convention.

Suggestion:

// Ensure that the fallback system didn't replace the contract.

---

crates/blockifier/src/execution/syscalls/syscall_tests/consts.rs line 1 at r1 (raw file):

pub const REQUIRED_GAS_CALL_CONTRACT_TEST: u64 = 105680;

Rename this file to constants.rs (naming convention)

---

crates/blockifier/src/execution/syscalls/syscall_tests/deploy.rs line 29 at r1 (raw file):

    );

    assert_consistent_contract_version(deployer_contract, &state);

We may not need these checks since we ain't going to support fallback to casm
Please add a TODO (can be on me) to share the init code of the tests in this file once assert_consistent_contract_version is removed.

Code quote:

assert_consistent_contract_version

---

crates/blockifier/src/execution/syscalls/syscall_tests/get_block_hash.rs line 84 at r1 (raw file):

    assert!(error
        .to_string()
        .contains("Unauthorized syscall get_block_hash in execution mode Validate"));

Seems redundant - remove one of the checks.

Code quote:

    check_entry_point_execution_error_for_custom_hint!(
        &error,
        "Unauthorized syscall get_block_hash in execution mode Validate.",
    );

    assert!(error
        .to_string()
        .contains("Unauthorized syscall get_block_hash in execution mode Validate"));

---

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 141 at r1 (raw file):

    let expected_resource_bounds: Vec<Felt> = match (test_contract, version) {
        (FeatureContract::LegacyTestContract, _) => vec![],
        (_, version) if version == TransactionVersion(Felt::from_hex("0x1").unwrap()) => vec![

Suggestion:

TransactionVersion::ONE

---

crates/blockifier/src/execution/syscalls/syscall_tests/get_execution_info.rs line 157 at r1 (raw file):

    let expected_tx_info: Vec<Felt>;
    let tx_info: TransactionInfo;
    if version == TransactionVersion(Felt::from_hex("0x1").unwrap()) {

Suggestion:

TransactionVersion::ONE

---

crates/blockifier/src/execution/syscalls/syscall_tests/storage_read_write.rs line 48 at r1 (raw file):

    assert_eq!(value_from_state, value);

    // ensure that the fallback system didn't replace the contract

Comment convention (start with a capital letter, end with .)

Suggestion:

// Ensure that the fallback system didn't replace the contract.

---

crates/blockifier/src/execution/syscalls/syscall_tests/utils.rs line 8 at r1 (raw file):

pub fn assert_consistent_contract_version(contract: FeatureContract, state: &dyn State) {
    let hash = contract.get_class_hash();

Suggestion:

class_hash = contract.get_class_hash();

---

crates/blockifier/src/execution/syscalls/syscall_tests/utils.rs line 18 at r1 (raw file):

        | FeatureContract::FaultyAccount(_)
        | FeatureContract::TestContract(_) => {
            // Assert contract uses VM

Comment convention

Suggestion:

// Assert contract uses VM.

[Yoni-Starkware]

Reviewable status: 14 of 19 files reviewed, 10 unresolved discussions (waiting on @varex83)

---

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

    };
    let error = entry_point_call.execute_directly(&mut state).unwrap_err().to_string();
    assert!(error.contains("Out of gas"));

(Non-blocking) why didn't you take this?

    assert_matches!(error, EntryPointExecutionError::ExecutionFailed{ error_data }
        if error_data == vec![felt!(OUT_OF_GAS_ERROR)]);

(It's more specific)

Code quote:

    assert!(error.contains("Out of gas"));

[varex83]

As far as I understood, we don't need the assert_consistent_contract_version at all, correct?

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

---

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

Previously, Yoni-Starkware (Yoni) wrote…

(Non-blocking) why didn't you take this?

    assert_matches!(error, EntryPointExecutionError::ExecutionFailed{ error_data }
        if error_data == vec![felt!(OUT_OF_GAS_ERROR)]);

(It's more specific)

We've done that because native vs VM errors will have different interfaces and the easiest way to implement error checking was using .contains

[Yoni-Starkware]

As far as I can tell - right.

Reviewed 11 of 11 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @varex83)

---

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

Previously, varex83 (Bohdan Ohorodnii) wrote…

We've done that because native vs VM errors will have different interfaces and the easiest way to implement error checking was using .contains

Cool

[Yoni-Starkware]

Reviewed 3 of 3 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @varex83)

[Yoni-Starkware]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @varex83)

[Yoni-Starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @varex83)

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 986 at r6 (raw file):

    let error = EntryPointExecutionError::ExecutionFailed { error_data };
    assert_eq!(error.to_string(), "Execution failed. Failure reason: \"Execution failure\".");
}

Sorry, missed that - please keep this test

Code quote:

#[test]
fn test_syscall_failure_format() {
    let error_data = vec![
        // Magic to indicate that this is a byte array.
        BYTE_ARRAY_MAGIC,
        // the number of full words in the byte array.
        "0x00",
        // The pending word of the byte array: "Execution failure"
        "0x457865637574696f6e206661696c757265",
        // The length of the pending word.
        "0x11",
    ]
    .into_iter()
    .map(|x| Felt::from_hex(x).unwrap())
    .collect();
    let error = EntryPointExecutionError::ExecutionFailed { error_data };
    assert_eq!(error.to_string(), "Execution failed. Failure reason: \"Execution failure\".");
}

[Yoni-Starkware]

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

[Yoni-Starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @varex83)

[Yoni-Starkware]

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