chore(blockifier): add struct ExecutionFlags to AccTx & use instead of transaction::ExecutionFlags

[avivg-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[avivg-starkware]

• chore(blockifier): access self.execution_flags instead of passing as args #2491
• chore(blockifier): rename fn account_invoke_tx to invoke_tx_with_default_flags #2479
• chore(blockifier): remove declare.rs deploy_account.rs invoke.rs from blockifier #2478
• chore(blockifier): remove struct ExecutionFlags and replace w concurrency_mode bool #2470
• chore(blockifier): remove only_qury from IvokeTxArgs #2437
• chore(blockifier): add new constructor to AccountTransaction #2431
• chore(blockifier): add struct ExecutionFlags to AccTx & use instead of transaction::ExecutionFlags #2429 👈 (View in Graphite)
• chore(blockifier): use test_utils::invoke_tx() instead of trans::test_utils::account_invoke_tx() #2428 : 1 other dependent PR (#2354 )
• chore(blockifier): invoke() declare() deploy_account() change ret val to api_tx #2412
• chore(blockifier): enforce_fee() impl by api::executable_transaction::AccountTransaction #2377
• chore(blockifier): explicit creation of AccountTransaction #2331
• main

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

[github-actions]

Artifacts upload workflows:

• Started at 2024-12-03T12:39:14Z
• Started at 2024-12-03T13:01:50Z
• Started at 2024-12-03T13:09:01Z
• Started at 2024-12-03T13:11:06Z
• Started at 2024-12-03T13:37:20Z
• Started at 2024-12-03T14:07:38Z
• Started at 2024-12-03T14:09:17Z
• Started at 2024-12-03T14:10:10Z
• Started at 2024-12-03T14:46:20Z
• Started at 2024-12-03T14:47:02Z
• Started at 2024-12-03T15:25:35Z
• Started at 2024-12-03T15:34:58Z
• Started at 2024-12-04T13:14:57Z
• Started at 2024-12-04T14:00:38Z
• Started at 2024-12-04T14:00:38Z
• Started at 2024-12-04T14:00:40Z
• Started at 2024-12-05T09:57:17Z
• Started at 2024-12-05T10:00:50Z
• Started at 2024-12-05T14:30:44Z
• Started at 2024-12-05T15:46:12Z
• Started at 2024-12-05T19:51:03Z
• Started at 2024-12-05T20:22:06Z

[codecov]

Codecov Report

Attention: Patch coverage is 53.40909% with 41 lines in your changes missing coverage. Please review.

Project coverage is 61.10%. Comparing base (e3165c4) to head (0557829).
Report is 727 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/native_blockifier/src/py_transaction.rs	0.00%	19 Missing ⚠️
...ution/src/state_reader/reexecution_state_reader.rs	0.00%	11 Missing ⚠️
crates/papyrus_execution/src/lib.rs	61.11%	6 Missing and 1 partial ⚠️
...lockifier/src/transaction/transaction_execution.rs	33.33%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2429       +/-   ##
===========================================
+ Coverage   40.10%   61.10%   +21.00%     
===========================================
  Files          26      206      +180     
  Lines        1895    22859    +20964     
  Branches     1895    22859    +20964     
===========================================
+ Hits          760    13969    +13209     
- Misses       1100     7894     +6794     
- Partials       35      996      +961     

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

[Yoni-Starkware]

Reviewed 1 of 20 files at r1, all commit messages.
Reviewable status: 1 of 22 files reviewed, 1 unresolved discussion (waiting on @avivg-starkware)

---

crates/native_blockifier/src/py_transaction.rs line 149 at r2 (raw file):

                ..ExecutionFlags::default()
            };
            AccountTransaction { tx, execution_flags }.into()

Save this code duplication - add a constructor that gets the tx.
new_for_sequencing or something like this

Code quote:

            let execution_flags = ExecutionFlags {
                only_query,
                charge_fee: enforce_fee(&tx, only_query),
                ..ExecutionFlags::default()
            };
            AccountTransaction { tx, execution_flags }.into()

[Yoni-Starkware]

Reviewed 1 of 13 files at r2.
Reviewable status: 2 of 22 files reviewed, 3 unresolved discussions (waiting on @avivg-starkware)

---

crates/starknet_gateway/src/stateful_transaction_validator.rs line 81 at r2 (raw file):

        let only_query = false;
        let charge_fee = enforce_fee(&executable_tx, only_query);
        let execution_flags = ExecutionFlags { only_query, charge_fee, validate: true };

Should it be !skip_validate?

Code quote:

validate: true }

---

crates/starknet_batcher/src/block_builder.rs line 177 at r2 (raw file):

                                charge_fee,
                                validate: true,
                            },

Use the new_for_sequencing c-tor

Code quote:

                        let only_query = false;
                        let charge_fee = enforce_fee(account_tx, only_query);
                        BlockifierTransaction::Account(AccountTransaction {
                            // TODO(yair): Avoid this clone.
                            tx: account_tx.clone(),
                            execution_flags: ExecutionFlags {
                                only_query,
                                charge_fee,
                                validate: true,
                            },

[Yoni-Starkware]

Reviewable status: 2 of 22 files reviewed, 4 unresolved discussions (waiting on @avivg-starkware)

---

crates/blockifier/src/blockifier/transaction_executor_test.rs line 136 at r2 (raw file):

    );
    let only_query = false;
    let charge_fee = enforce_fee(&declare_tx, only_query);

What is this func? are you sure it is necessary?
Seems like you can use the new_for_sequencing c-tor everywhere here

Code quote:

enforce_fee

[avivg-starkware]

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

---

crates/blockifier/src/blockifier/transaction_executor_test.rs line 136 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

What is this func? are you sure it is necessary?
Seems like you can use the new_for_sequencing c-tor everywhere here

enforce_fee is the func created in the prev PR to allow computing 'charge_fee' before creating AccountTransaction #2377

and defiantly for using 'new_for_sequencing', in the following PR: #2431

---

crates/native_blockifier/src/py_transaction.rs line 149 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Save this code duplication - add a constructor that gets the tx.
new_for_sequencing or something like this

Defiantly! added in the next PR as I'm anyway creating a constructor there. #2431

---

crates/starknet_batcher/src/block_builder.rs line 177 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Use the new_for_sequencing c-tor

Done in #2431

---

crates/starknet_gateway/src/stateful_transaction_validator.rs line 81 at r2 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Should it be !skip_validate?

Seems right. thank you.

[Yoni-Starkware]

Reviewed 6 of 20 files at r1, 10 of 13 files at r2, 5 of 5 files at r5, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @avivg-starkware)

---

crates/blockifier/src/transaction/transactions_test.rs line 1976 at r5 (raw file):

        validate_constructor,
        validate: true,
        charge_fee: false, // We test `__validate__`, and don't care about the cahrge fee flow.

Suggestion:

charge fee flow

---

crates/blockifier/src/transaction/account_transaction.rs line 724 at r5 (raw file):

        state: &mut TransactionalState<'_, U>,
        block_context: &BlockContext,
        execution_flags_: TransactionExecutionFlags,

Consider passing concurrecy_mode without this struct

Code quote:

execution_flags_: TransactionExecutionFlags,

---

crates/blockifier/src/transaction/account_transaction.rs line 756 at r5 (raw file):

            tx_context.clone(),
            self.execution_flags.validate,
            self.execution_flags.charge_fee,

Be sure to push these inside instead of passing them as arguments (now that they are part of self)

Code quote:

            self.execution_flags.validate,
            self.execution_flags.charge_fee,

[avivg-starkware]

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

---

crates/blockifier/src/transaction/account_transaction.rs line 724 at r5 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Consider passing concurrecy_mode without this struct

added a PR #2470

---

crates/blockifier/src/transaction/account_transaction.rs line 756 at r5 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Be sure to push these inside instead of passing them as arguments (now that they are part of self)

WOW, yes. will do

---

crates/blockifier/src/transaction/transactions_test.rs line 1976 at r5 (raw file):

        validate_constructor,
        validate: true,
        charge_fee: false, // We test `__validate__`, and don't care about the cahrge fee flow.

Done

[Yoni-Starkware]

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

[avivg-starkware]

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

---

crates/blockifier/src/transaction/account_transaction.rs line 756 at r5 (raw file):

Previously, avivg-starkware wrote…

WOW, yes. will do

#2491

[avivg-starkware]

Reviewed 6 of 20 files at r1, 8 of 13 files at r2, 2 of 5 files at r5, 6 of 6 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @avivg-starkware)

[avivg-starkware]

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