chore: split the inner tx creator in blockifier test utils

[ArniStarkware]

This change is 

[ArniStarkware]

• chore: move chain_id create for testing to snapi test utils #666 : 3 dependent PRs (#672 , #694 , #701 )
• chore: move transaction creator test util to starknet api #661
• chore: split the inner tx creator in blockifier test utils #660 👈
• main

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

Join @ArniStarkware and the rest of your teammates on Graphite

[codecov]

Codecov Report

Attention: Patch coverage is 98.61111% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.05%. Comparing base (c65f0b9) to head (4da7b0c).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/blockifier/src/test_utils/declare.rs	97.95%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #660      +/-   ##
==========================================
- Coverage   75.83%   71.05%   -4.79%     
==========================================
  Files         352       85     -267     
  Lines       37423    10944   -26479     
  Branches    37423    10944   -26479     
==========================================
- Hits        28381     7776   -20605     
+ Misses       6726     2780    -3946     
+ Partials     2316      388    -1928     

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

[MohammadNassar1]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArniStarkware)

---

crates/blockifier/src/test_utils/declare.rs line 78 at r1 (raw file):

}

fn inner_declare_tx(

Please change the order of the functions.
First, public methods, then private functions.

Also relevant for the below files.

Code quote:

fn inner_declare_tx(

[MohammadNassar1]

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

---

crates/blockifier/src/test_utils/declare.rs line 78 at r1 (raw file):

Previously, MohammadNassar1 (mohammad-starkware) wrote…

Please change the order of the functions.
First, public methods, then private functions.

Also relevant for the below files.

If you plan to move this function to the sn-api crate, there is no need to reorder them in this PR.

[ArniStarkware]

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @MohammadNassar1)

---

crates/blockifier/src/test_utils/declare.rs line 78 at r1 (raw file):

Previously, MohammadNassar1 (mohammad-starkware) wrote…

If you plan to move this function to the sn-api crate, there is no need to reorder them in this PR.

already done :) but TY!

[MohammadNassar1]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArniStarkware)

---

crates/blockifier/src/test_utils/invoke.rs line 83 at r2 (raw file):

    let default_tx_hash = TransactionHash::default();
    match only_query {

No need for additional variable.

Suggestion:

    let invoke_tx = inner_invoke_tx(invoke_args);

    let default_tx_hash = TransactionHash::default();
    match invoke_args.only_query {

[MohammadNassar1]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ArniStarkware)

---

crates/blockifier/src/test_utils/deploy_account.rs line 88 at r2 (raw file):

    let tx = inner_deploy_account_tx(deploy_tx_args, nonce_manager.next(contract_address));

    DeployAccountTransaction::new(tx, TransactionHash::default(), contract_address)

Do you know why the invoke and deploy account do not have a tx hash field, while declare does?

Code quote:

TransactionHash::default()

[ArniStarkware]

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @MohammadNassar1)

---

crates/blockifier/src/test_utils/deploy_account.rs line 88 at r2 (raw file):

Previously, MohammadNassar1 (mohammad-starkware) wrote…

Do you know why the invoke and deploy account do not have a tx hash field, while declare does?

I don't know. Let us keep the option available and streamline the util across tx types.
I did not find why this util is used in declare - I see no instance of it's use (I deleted it and the blockifier's tests passed).

---

crates/blockifier/src/test_utils/invoke.rs line 83 at r2 (raw file):

Previously, MohammadNassar1 (mohammad-starkware) wrote…

No need for additional variable.

Your suggestion yields this error.

error[E0382]: use of moved value: `invoke_args`
  --> crates/blockifier/src/test_utils/invoke.rs:82:5
   |
78 | pub fn invoke_tx(invoke_args: InvokeTxArgs) -> InvokeTransaction {
   |                  ----------- move occurs because `invoke_args` has type `test_utils::invoke::InvokeTxArgs`, which does not implement the `Copy` trait
79 |     let invoke_tx = inner_invoke_tx(invoke_args);
   |                                     ----------- value moved here
...
82 |     match invoke_args.only_query {

#rust_things #blessed.

[MohammadNassar1]

Reviewed 1 of 3 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArniStarkware)

---

crates/blockifier/src/test_utils/invoke.rs line 83 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Your suggestion yields this error.

error[E0382]: use of moved value: `invoke_args`
  --> crates/blockifier/src/test_utils/invoke.rs:82:5
   |
78 | pub fn invoke_tx(invoke_args: InvokeTxArgs) -> InvokeTransaction {
   |                  ----------- move occurs because `invoke_args` has type `test_utils::invoke::InvokeTxArgs`, which does not implement the `Copy` trait
79 |     let invoke_tx = inner_invoke_tx(invoke_args);
   |                                     ----------- value moved here
...
82 |     match invoke_args.only_query {

#rust_things #blessed.

OMG

[ArniStarkware]

Merge activity

• Sep 8, 3:01 AM EDT: @ArniStarkware started a stack merge that includes this pull request via Graphite.
• Sep 8, 3:01 AM EDT: @ArniStarkware merged this pull request with Graphite.