refactor(blockifier): remove execution_resources arg

[Yoni-Starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload triggered. View details here

[codecov]

Codecov Report

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

Project coverage is 77.29%. Comparing base (e3165c4) to head (5af6129).
Report is 521 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/blockifier/src/transaction/transactions.rs	83.33%	0 Missing and 2 partials ⚠️
...lockifier/src/transaction/transaction_execution.rs	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2189       +/-   ##
===========================================
+ Coverage   40.10%   77.29%   +37.18%     
===========================================
  Files          26      385      +359     
  Lines        1895    40367    +38472     
  Branches     1895    40367    +38472     
===========================================
+ Hits          760    31200    +30440     
- Misses       1100     6866     +5766     
- Partials       35     2301     +2266     

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

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests
• JS Bundle Analysis - Avoid shipping oversized bundles

[dorimedini-starkware]

Reviewed 20 of 20 files at r1, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @TzahiTaub and @Yoni-Starkware)

---

crates/blockifier/src/execution/call_info.rs line 201 at r1 (raw file):

        ExecutionSummary {
            charged_resources: self.charged_resources.clone(),

why is this field precomputed, and not computed in summarize like everything else?

Code quote:

charged_resources: self.charged_resources.clone(),

---

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

        },
        charged_resources: ChargedResources {
            // TODO(Yoni): fix similarly to the VM - share that code.

can you be more specific? assume people read this without context

Code quote:

// TODO(Yoni): fix similarly to the VM - share that code.

---

crates/blockifier/src/transaction/objects_test.rs line 195 at r1 (raw file):

    let expected_summary = ExecutionSummary {
        charged_resources: ChargedResources::default(),

why do we expect zero charged resources in this test...? seems like it should be positive

Code quote:

charged_resources: ChargedResources::default(),

---

crates/blockifier/src/fee/receipt.rs line 62 at r1 (raw file):

            reverted_steps,
        } = tx_receipt_params;
        let charged_resources = execution_summary_without_fee_transfer.charged_resources.clone();

don't we add fee transfer overhead when we charge a fee? isn't this a breaking change?

Code quote:

let charged_resources = execution_summary_without_fee_transfer.charged_resources.clone();

---

crates/blockifier/src/execution/entry_point_execution.rs line 443 at r1 (raw file):

            acc
        },
    );

same remark as above

Code quote:

    let charged_resources = syscall_handler.inner_calls.iter().fold(
        charged_resources_without_inner_calls,
        |mut acc, inner_call| {
            acc += &inner_call.charged_resources;
            acc
        },
    );

---

crates/blockifier/src/execution/deprecated_entry_point_execution.rs line 261 at r1 (raw file):

            acc
        },
    );

isn't there a derive_more::Sum you can annotate resource with?
or, make this change?

Suggestion:

    let full_vm_resources = vm_resources_without_inner_calls + syscall_handler
        .inner_calls
        .iter()
        .map(|inner_call| inner_call.charged_resources.vm_resources)
        .sum();

[Yoni-Starkware]

Reviewable status: 13 of 21 files reviewed, 6 unresolved discussions (waiting on @dorimedini-starkware and @TzahiTaub)

---

crates/blockifier/src/execution/call_info.rs line 201 at r1 (raw file):

Previously, dorimedini-starkware wrote…

why is this field precomputed, and not computed in summarize like everything else?

The resources include inner calls. Events, message, etc - don't.

---

crates/blockifier/src/execution/deprecated_entry_point_execution.rs line 261 at r1 (raw file):

Previously, dorimedini-starkware wrote…

isn't there a derive_more::Sum you can annotate resource with?
or, make this change?

ExecutionResources is not part of our repo, so I can't impl this trait.

---

crates/blockifier/src/execution/entry_point_execution.rs line 443 at r1 (raw file):

Previously, dorimedini-starkware wrote…

same remark as above

Same

---

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

Previously, dorimedini-starkware wrote…

can you be more specific? assume people read this without context

Easier to fix this. Done

---

crates/blockifier/src/fee/receipt.rs line 62 at r1 (raw file):

Previously, dorimedini-starkware wrote…

don't we add fee transfer overhead when we charge a fee? isn't this a breaking change?

Didn't change anything.
The fee transfer overhead (and others) is part of the transaction overhead, which is added later in this func.

---

crates/blockifier/src/transaction/objects_test.rs line 195 at r1 (raw file):

Previously, dorimedini-starkware wrote…

why do we expect zero charged resources in this test...? seems like it should be positive

No, need to add it to the test. Done.

[github-actions]

Artifacts upload triggered. View details here

[dorimedini-starkware]

Reviewed 8 of 8 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @TzahiTaub and @Yoni-Starkware)

---

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

    };
    let mut charged_resources = charged_resources_without_inner_calls;
    charged_resources += &CallInfo::summarize_charged_resources(syscall_handler.inner_calls.iter());

Suggestion:

    let charged_resources = charged_resources_without_inner_calls +
        &CallInfo::summarize_charged_resources(syscall_handler.inner_calls.iter());

---

crates/blockifier/src/execution/entry_point_execution.rs line 438 at r2 (raw file):

    };
    let mut charged_resources = charged_resources_without_inner_calls;
    charged_resources += &CallInfo::summarize_charged_resources(syscall_handler.inner_calls.iter());

Suggestion:

    let charged_resources = charged_resources_without_inner_calls +
        &CallInfo::summarize_charged_resources(syscall_handler.inner_calls.iter());

---

crates/blockifier/src/execution/deprecated_entry_point_execution.rs line 262 at r2 (raw file):

    };
    let mut charged_resources = charged_resources_without_inner_calls;
    charged_resources += &CallInfo::summarize_charged_resources(syscall_handler.inner_calls.iter());

Suggestion:

    let charged_resources = charged_resources_without_inner_calls +
        &CallInfo::summarize_charged_resources(syscall_handler.inner_calls.iter());

---

crates/blockifier/src/execution/call_info.rs line 222 at r2 (raw file):

    ) -> ChargedResources {
        // Note: the charged resourses of a call contains the inner call resources, unlike other
        // fields such as events and messages,

is it possible to change this, so that charged resources behave like other fields?
it's still confusing - i.e. why would you need a summarize method like this, for a field that "already includes inner calls"?

Code quote:

        // Note: the charged resourses of a call contains the inner call resources, unlike other
        // fields such as events and messages,

---

crates/blockifier/src/execution/syscalls/syscall_tests/library_call.rs line 0 at r2 (raw file):
why are the native values now zero? this seems like an error

[Yoni-Starkware]

Reviewable status: 16 of 22 files reviewed, 4 unresolved discussions (waiting on @dorimedini-starkware and @TzahiTaub)

---

crates/blockifier/src/execution/call_info.rs line 222 at r2 (raw file):

Previously, dorimedini-starkware wrote…

is it possible to change this, so that charged resources behave like other fields?
it's still confusing - i.e. why would you need a summarize method like this, for a field that "already includes inner calls"?

We thought about it, but let's not do it in this PR. It will break many things (os, feeder)
I'll ask Ariel

---

crates/blockifier/src/execution/syscalls/syscall_tests/library_call.rs line at r2 (raw file):

Previously, dorimedini-starkware wrote…

why are the native values now zero? this seems like an error

I zeroed it sine setting gas_for_fee = gas_consumed was a mistake. The gas for fee for native should be handled exactly the same as the VM. @TzahiTaub should do it in his PR.

---

crates/blockifier/src/execution/entry_point_execution.rs line 438 at r2 (raw file):

    };
    let mut charged_resources = charged_resources_without_inner_calls;
    charged_resources += &CallInfo::summarize_charged_resources(syscall_handler.inner_calls.iter());

Done.

---

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

    };
    let mut charged_resources = charged_resources_without_inner_calls;
    charged_resources += &CallInfo::summarize_charged_resources(syscall_handler.inner_calls.iter());

Done.

---

crates/blockifier/src/execution/deprecated_entry_point_execution.rs line 262 at r2 (raw file):

    };
    let mut charged_resources = charged_resources_without_inner_calls;
    charged_resources += &CallInfo::summarize_charged_resources(syscall_handler.inner_calls.iter());

Done.

[github-actions]

Artifacts upload triggered. View details here

[dorimedini-starkware]

Reviewed 5 of 6 files at r3, all commit messages.
Reviewable status: 21 of 22 files reviewed, all discussions resolved (waiting on @TzahiTaub)

[github-actions]

Artifacts upload triggered. View details here

[dorimedini-starkware]

Reviewed 1 of 6 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @TzahiTaub and @Yoni-Starkware)

---

crates/blockifier/cairo_native line 1 at r4 (raw file):

Subproject commit 0bc6f408884370e4fbbf421c4e8e109911c3d73e

this shouldn't be here

Code quote:

Subproject commit 0bc6f408884370e4fbbf421c4e8e109911c3d73e

[Yoni-Starkware]

Reviewed 2 of 2 files at r4, 1 of 1 files at r5.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @dorimedini-starkware and @TzahiTaub)

---

crates/blockifier/cairo_native line 1 at r4 (raw file):

Previously, dorimedini-starkware wrote…

this shouldn't be here

Done.

[github-actions]

Artifacts upload triggered. View details here

[dorimedini-starkware]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @TzahiTaub)

[Yoni-Starkware]

Reviewed 12 of 20 files at r1, 3 of 8 files at r2, 5 of 6 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @TzahiTaub)