chore(blockifier): add a check on deploy syscall gas cost in the happ…

[meship-starkware]

…y flows of deploy tests

---

This change is 

[avi-starkware]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @meship-starkware and @noaov1)

---

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

        deploy_constractor_call.execution,
        CallExecution { retdata: retdata![], gas_consumed: 0, ..CallExecution::default() }
    );

Btw, why do we even need to look at the call info of the constructor call? If it has 0 gas_consumed anyway, shouldn't we just remove this part?

Suggestion:

    // Note, this is the CallInfo of the constructor call. Meaning the CallExecution is the
    // execution of the constructor and not the deploy syscall.
    let deploy_constructor_call = &deploy_call.inner_calls[0];

    assert_eq!(deploy_constructor_call.call.storage_address, deployed_contract_address);
    assert_eq!(
        deploy_constructor_call.execution,
        CallExecution { retdata: retdata![], gas_consumed: 0, ..CallExecution::default() }
    );

---

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

    deployer_contract: FeatureContract,
    expected_gas: u64,
    expected_constractor_gas: u64,

Suggestion:

    expected_constructor_gas: u64,

---

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

            gas_consumed: expected_constractor_gas,
            ..CallExecution::default()
        }

Suggestion:

    // execution of the constructor and not the deploy syscall.
    let deploy_constructor_call = &deploy_call.inner_calls[0];

    assert_eq!(deploy_constructor_call.call.storage_address, contract_address);
    assert_eq!(
        deploy_constructor_call.execution,
        CallExecution {
            // The test contract constructor returns its first argument.
            retdata: retdata![constructor_calldata[0]],
            // This reflects the gas cost of storage write syscall.
            gas_consumed: expected_constructor_gas,
            ..CallExecution::default()
        }

[meship-starkware]

Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @ArniStarkware, @avi-starkware, and @noaov1)

---

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

Previously, avi-starkware wrote…

Btw, why do we even need to look at the call info of the constructor call? If it has 0 gas_consumed anyway, shouldn't we just remove this part?

It's a good question. I am not sure; I think Arni wrote this test, maybe it's worth asking him.
@ArniStarkware WDYT?
I think the objective is to confirm that if we are not calling the constructor the gas_cost is zero

---

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

    deployer_contract: FeatureContract,
    expected_gas: u64,
    expected_constractor_gas: u64,

Done.

---

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

            gas_consumed: expected_constractor_gas,
            ..CallExecution::default()
        }

Done.

[ArniStarkware]

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

Previously, meship-starkware (Meshi Peled) wrote…

It's a good question. I am not sure; I think Arni wrote this test, maybe it's worth asking him.
@ArniStarkware WDYT?
I think the objective is to confirm that if we are not calling the constructor the gas_cost is zero

Talked F2f.
We want to check that the inner call consumes no gas, but we do want to check that the Tx at large consumes some gas.

[avi-starkware]

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

[avi-starkware]

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

[noaov1]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @meship-starkware)

---

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

    // Note, this is the CallInfo of the constructor call. Meaning the CallExecution is the
    // execution of the constructor and not the deploy syscall.

I think that this can be removed. Same below.

Code quote:

    // Note, this is the CallInfo of the constructor call. Meaning the CallExecution is the
    // execution of the constructor and not the deploy syscall.

---

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

    // Note, this is the CallInfo of the constructor call. Meaning the CallExecution is the
    // execution of the constructor and not the deploy syscall.
    let deploy_constructor_call = &deploy_call.inner_calls[0];

Same below.

Suggestion:

let constructor_call = &deploy_call.inner_calls[0];

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.45%. Comparing base (be0f73a) to head (1ffcc7f).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #478      +/-   ##
==========================================
+ Coverage   76.42%   76.45%   +0.03%     
==========================================
  Files         349      349              
  Lines       36939    36962      +23     
  Branches    36939    36962      +23     
==========================================
+ Hits        28231    28260      +29     
+ Misses       6383     6380       -3     
+ Partials     2325     2322       -3     

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

[avi-starkware]

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