refactor(blockifier): impl .into() from api executable tx to blockifier AccountTransaction

[avivg-starkware]

…tx to account_transaction

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.66%. Comparing base (e3165c4) to head (0930b05).
Report is 295 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1868       +/-   ##
===========================================
+ Coverage   40.10%   67.66%   +27.55%     
===========================================
  Files          26      102       +76     
  Lines        1895    13872    +11977     
  Branches     1895    13872    +11977     
===========================================
+ Hits          760     9386     +8626     
- Misses       1100     4085     +2985     
- Partials       35      401      +366     

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

[meship-starkware]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArniStarkware and @noaov1)

[ArniStarkware]

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

---

crates/blockifier/src/test_utils/deploy_account.rs line 28 at r1 (raw file):

        DeployAccountTransaction::new(deploy_account_tx, tx_hash, contract_address);

    executable_deploy_account_tx.into()

use starknet_api::test_utils::deploy_account::executable_deploy_account_tx

Same for the declare (wow!).
It's not the same for the invoke - add a todo over there...

Suggestion:

    let executable_deploy_account_tx =
        executable_deploy_account_tx(..);

    executable_deploy_account_tx.into()

---

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

    let tx_hash = declare_tx_args.tx_hash;
    let declare_tx = starknet_api::test_utils::declare::declare_tx(declare_tx_args);
    let executable_declare = DeclareTransaction::new(declare_tx, tx_hash, class_info).unwrap();

Good job!

Code quote:

let executable_declare = DeclareTransaction::new(declare_tx, tx_hash, class_info).unwrap();

---

crates/blockifier/src/transaction/account_transaction.rs line 80 at r1 (raw file):

/// Represents a paid Starknet transaction.
#[derive(Clone, Debug, derive_more::From)]

This did not work? Didn't it do what these from methods did?

Code quote:

, derive_more::From

[ArniStarkware]

Reviewed 1 of 5 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @avivg-starkware and @noaov1)

[github-actions]

Artifacts upload triggered. View details here

[avivg-starkware]

Reviewable status: 4 of 5 files reviewed, 2 unresolved discussions (waiting on @ArniStarkware, @meship-starkware, and @noaov1)

---

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

Previously, ArniStarkware (Arnon Hod) wrote…

Good job!

:)

---

crates/blockifier/src/test_utils/deploy_account.rs line 28 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

use starknet_api::test_utils::deploy_account::executable_deploy_account_tx

Same for the declare (wow!).
It's not the same for the invoke - add a todo over there...

Will use the code SUGGESTION in #1688.

What is the change you mean for the invoke?

---

crates/blockifier/src/transaction/account_transaction.rs line 80 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

This did not work? Didn't it do what these from methods did?

Sorry, works! there is a test that sometimes fails for me after making a change and running for the 1st time- sync_happy_flow(). are you familiar with this perhaps?

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[ArniStarkware]

crates/blockifier/src/transaction/account_transaction.rs line 80 at r1 (raw file):

Previously, avivg-starkware wrote…

Sorry, works! there is a test that sometimes fails for me after making a change and running for the 1st time- sync_happy_flow(). are you familiar with this perhaps?

Don't know what you are talking about :o

[ArniStarkware]

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

[ArniStarkware]

Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @avivg-starkware and @noaov1)

---

crates/blockifier/src/test_utils/deploy_account.rs line 28 at r1 (raw file):

Previously, avivg-starkware wrote…

Will use the code SUGGESTION in #1688.

What is the change you mean for the invoke?

I don't understand.

1. Which PR came before which PR?
2. I don't see my suggestion implemented here or there.
3. This PR is small as it is, and this suggestion is small - you can follow through with my suggestion here and close this subject.

[avivg-starkware]

Reviewable status: 3 of 5 files reviewed, 1 unresolved discussion (waiting on @ArniStarkware, @meship-starkware, and @noaov1)

---

crates/blockifier/src/test_utils/deploy_account.rs line 28 at r1 (raw file):

Previously, avivg-starkware wrote…

executable_deploy_account_tx, returns a DeployAccountTransaction that is api executable,
currently here, DeployAccountTransaction is a blockifier struct.
After the PR i mentioned, the DeployAccountTransaction used here, will be the api executable.
Does it answer? am I missing something?

Added a comment

[ArniStarkware]

crates/blockifier/src/test_utils/deploy_account.rs line 28 at r4 (raw file):

    // create executable_deploy_account_tx instead of code above.
    let executable_deploy_account_tx =
        DeployAccountTransaction::new(deploy_account_tx, tx_hash, contract_address);

Suggestion:

    // TODO(AvivG): use starknet_api::test_utils::deploy_account::executable_deploy_account_tx to
    // create executable_deploy_account_tx.
    let executable_deploy_account_tx =
        DeployAccountTransaction::new(deploy_account_tx, tx_hash, contract_address);

[ArniStarkware]

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

Previously, avivg-starkware wrote…

thanks I see now. Added a TODO. looks clear?

Yes.

[ArniStarkware]

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

---

crates/blockifier/src/test_utils/deploy_account.rs line 28 at r1 (raw file):

Previously, avivg-starkware wrote…

Added a comment

I still do not understand what you mean, but following through on the to-do list will solve the issue I mentioned.

---

crates/blockifier/src/test_utils/declare.rs line 10 at r4 (raw file):

    let tx_hash = declare_tx_args.tx_hash;
    let declare_tx = starknet_api::test_utils::declare::declare_tx(declare_tx_args);
    let executable_declare = DeclareTransaction::new(declare_tx, tx_hash, class_info).unwrap();

Suggestion:

// TODO(AvivG): use starknet_api::test_utils::declare::executable_declare_tx to
// create executable_declare.
let executable_declare = DeclareTransaction::new(declare_tx, tx_hash, class_info).unwrap();

[meship-starkware]

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

[avivg-starkware]

• refactor(blockifier): impl .into() from api executable tx to blockifier AccountTransaction #1868 👈
• refactor(blockifier): change return type of invoke_tx deploy_account_tx to account_transaction #1643
• main

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

Join @avivg-starkware and the rest of your teammates on Graphite

[github-actions]

Artifacts upload triggered. View details here

[meship-starkware]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @avivg-starkware and @noaov1)

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[github-actions]

Artifacts upload triggered. View details here

[avivg-starkware]

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

---

crates/blockifier/src/test_utils/deploy_account.rs line 28 at r4 (raw file):

    // create executable_deploy_account_tx instead of code above.
    let executable_deploy_account_tx =
        DeployAccountTransaction::new(deploy_account_tx, tx_hash, contract_address);

Done.

---

crates/blockifier/src/test_utils/declare.rs line 10 at r4 (raw file):

    let tx_hash = declare_tx_args.tx_hash;
    let declare_tx = starknet_api::test_utils::declare::declare_tx(declare_tx_args);
    let executable_declare = DeclareTransaction::new(declare_tx, tx_hash, class_info).unwrap();

Done.

[meship-starkware]

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

[ArniStarkware]

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

[meship-starkware]

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