chore: move transaction creator test util to starknet api

[ArniStarkware]

This is a step towards addressing the issue in: #519.

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 95.80838% with 7 lines in your changes missing coverage. Please review.

Project coverage is 75.89%. Comparing base (c65f0b9) to head (6b965ef).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/starknet_api/src/test_utils/invoke.rs	89.36%	5 Missing ⚠️
crates/starknet_api/src/test_utils/declare.rs	98.36%	1 Missing ⚠️
...ates/starknet_api/src/test_utils/deploy_account.rs	97.82%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #661      +/-   ##
==========================================
+ Coverage   75.83%   75.89%   +0.05%     
==========================================
  Files         352       88     -264     
  Lines       37423    10935   -26488     
  Branches    37423    10935   -26488     
==========================================
- Hits        28381     8299   -20082     
+ Misses       6726     2227    -4499     
+ Partials     2316      409    -1907     

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

[MohammadNassar1]

Reviewed 7 of 24 files at r1, 17 of 17 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ArniStarkware and @dafnamatsry)

---

crates/blockifier/src/test_utils/declare.rs line 23 at r3 (raw file):

#[derive(Clone)]
pub struct DeclareTxArgs {

I assume no code changes.
You're only moving the code, right?

Code quote:

pub struct DeclareTxArgs {

---

crates/blockifier/src/test_utils/invoke.rs line 11 at r3 (raw file):

    let only_query = invoke_args.only_query;
    // TODO: Make TransactionVersion an enum and use match here.
    let invoke_tx = if invoke_args.version == TransactionVersion::ZERO {

Could you please explain why you kept this code here instead of moving it to sn-api and simply calling the create function from there?

---

crates/starknet_api/src/test_utils.rs line 8 at r3 (raw file):

// TODO: Default testing bounds should probably be AllResourceBounds variant.
pub fn default_testing_resource_bounds() -> ValidResourceBounds {

Maybe it can be in the same file of ValidResourceBound struct.

Code quote:

default_testing_resource_bounds(

[ArniStarkware]

Reviewable status: 15 of 25 files reviewed, 2 unresolved discussions (waiting on @dafnamatsry and @MohammadNassar1)

---

crates/blockifier/src/test_utils/declare.rs line 23 at r3 (raw file):

Previously, MohammadNassar1 (mohammad-starkware) wrote…

I assume no code changes.
You're only moving the code, right?

Indeed.

---

crates/blockifier/src/test_utils/invoke.rs line 11 at r3 (raw file):

Previously, MohammadNassar1 (mohammad-starkware) wrote…

Could you please explain why you kept this code here instead of moving it to sn-api and simply calling the create function from there?

Explanation: The function selector_from_name is from the blockifier crate.
I did not look much into moving it into the shared test util.
(note - selector_from_name(EXECUTE_ENTRY_POINT_NAME) is just a constant number).

On the other hand, v0 will be deleted anyway - so I did not want to work hard to support it.

---

crates/starknet_api/src/test_utils.rs line 8 at r3 (raw file):

Previously, MohammadNassar1 (mohammad-starkware) wrote…

Maybe it can be in the same file of ValidResourceBound struct.

Done.

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [65.695 ms 66.184 ms 67.085 ms]
change: [-7.9872% -4.7144% -1.5721%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
4 (4.00%) high mild
3 (3.00%) high severe

[github-actions]

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [66.962 ms 69.344 ms 72.460 ms]
change: [+1.4713% +4.1654% +9.9926%] (p = 0.04 < 0.05)
Performance has regressed.
Found 10 outliers among 100 measurements (10.00%)
2 (2.00%) high mild
8 (8.00%) high severe

[MohammadNassar1]

Reviewed 4 of 10 files at r4, 5 of 5 files at r5, all commit messages.
Reviewable status: 24 of 25 files reviewed, 1 unresolved discussion (waiting on @ArniStarkware and @dafnamatsry)

---

crates/starknet_api/src/test_utils.rs line 3 at r5 (raw file):

pub mod declare;
pub mod deploy_account;
pub mod invoke;

Why you define the modules here, and not in lib file?

Code quote:

pub mod declare;
pub mod deploy_account;
pub mod invoke;

[ArniStarkware]

Reviewable status: 24 of 25 files reviewed, 1 unresolved discussion (waiting on @dafnamatsry and @MohammadNassar1)

---

crates/starknet_api/src/test_utils.rs line 3 at r5 (raw file):

Previously, MohammadNassar1 (mohammad-starkware) wrote…

Why you define the modules here, and not in lib file?

I copied the folder/file structure from blockifier's test utils.

It is nice that the lib file points to the public-facing test_utils.rs, and here - it points to the inner workings of the util. also we don't have to decorate everything with: #[cfg(any(feature = "testing", test))] - but only once.

[MohammadNassar1]

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

[ArniStarkware]

Merge activity

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