refactor: refactor the stateful tx validator to get executable tx as input

[ArniStarkware]

In this PR we change the inputs for the run_validate method of the stateful validator. This also streamlines process+tx.

This change is 

[ArniStarkware]

• refactor: refactor the stateful tx validator to get executable tx as input #519 : 4 dependent PRs (#520 , #521 , #604 and 1 other) 👈
• chore: create executable tx creator for testing #672
• chore: move chain_id create for testing to snapi test utils #666 : 2 other dependent PRs (#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

[dafnamatsry]

Reviewed 2 of 5 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ArniStarkware, @ayeletstarkware, @MohammadNassar1, and @yair-starkware)

---

crates/gateway/src/stateful_transaction_validator.rs line 77 at r2 (raw file):

        mut validator: V,
    ) -> StatefulTransactionValidatorResult<ValidateInfo> {
        let account_tx = AccountTransaction::try_from(executable_tx.clone()).map_err(|error| {

Why the clone here? Can't AccountTransaction::try_from get a reference?

Code quote:

executable_tx.clone()

---

crates/gateway/src/stateful_transaction_validator.rs line 78 at r2 (raw file):

    ) -> StatefulTransactionValidatorResult<ValidateInfo> {
        let account_tx = AccountTransaction::try_from(executable_tx.clone()).map_err(|error| {
            error!("Failed to executable transaction into account transaction: {}", error);

Suggestion:

Failed to convert executable transaction into account transaction: {}

---

crates/gateway/src/stateful_transaction_validator_test.rs line 81 at r2 (raw file):

#[case::invalid_tx(invoke_tx(CairoVersion::Cairo1), Err(STATEFUL_VALIDATOR_FEE_ERROR))]
fn test_stateful_tx_validator(
    #[case] external_tx: RpcTransaction,

I think the test should start with an executable tx, and not do any conversion, and especially not compile anything.

Code quote:

#[case] external_tx: RpcTransaction,

[ArniStarkware]

crates/gateway/src/stateful_transaction_validator.rs line 78 at r2 (raw file):

    ) -> StatefulTransactionValidatorResult<ValidateInfo> {
        let account_tx = AccountTransaction::try_from(executable_tx.clone()).map_err(|error| {
            error!("Failed to executable transaction into account transaction: {}", error);

Done.

[ArniStarkware]

Reviewable status: 4 of 10 files reviewed, 1 unresolved discussion (waiting on @ayeletstarkware, @dafnamatsry, @MohammadNassar1, and @yair-starkware)

---

crates/gateway/src/gateway_test.rs line 161 at r8 (raw file):

Previously, dafnamatsry wrote…

Can you explain?

No. I would rather delete this function :)

This was a test util that should be deprecated.
Let us update the test.

[ArniStarkware]

Reviewable status: 4 of 10 files reviewed, 1 unresolved discussion (waiting on @ayeletstarkware, @dafnamatsry, @MohammadNassar1, and @yair-starkware)

---

crates/gateway/src/stateful_transaction_validator_test.rs line 67 at r9 (raw file):

    executable_invoke_tx(CairoVersion::Cairo1),
    Ok(ValidateInfo{
        tx_hash: TransactionHash::default(),

This is not the subject of this test.
This value is deleted in #521.

Code quote:

        tx_hash: TransactionHash::default(),

[ArniStarkware]

crates/gateway/src/gateway_test.rs line 161 at r8 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

No. I would rather delete this function :)

This was a test util that should be deprecated.
Let us update the test.

The original thread was about a test util called rpc_tx_to_account_tx that first appeared in r4 of this review session.
A new design of the code makes this code obsolete.

[yair-starkware]

Reviewed 1 of 6 files at r7, 3 of 8 files at r8, 5 of 5 files at r11, all commit messages.
Reviewable status: 9 of 10 files reviewed, 1 unresolved discussion (waiting on @ayeletstarkware, @dafnamatsry, and @MohammadNassar1)

[ArniStarkware]

Reviewable status: 9 of 10 files reviewed, 2 unresolved discussions (waiting on @ayeletstarkware, @dafnamatsry, and @MohammadNassar1)

a discussion (no related file):
In this PR we change the

---

[ArniStarkware]

Reviewable status: 9 of 10 files reviewed, 1 unresolved discussion (waiting on @ayeletstarkware, @dafnamatsry, and @MohammadNassar1)

a discussion (no related file):
The original goal of this PR was to refactor the stateful tx validator so that it receives an executable transaction as input.
Another goal achieved is that the proper executable transaction is now used as the mempool input. This change occurred to address issues with the gateway_test: test_add_tx. In the meantime we also cleaned some test utils for this test and for the stateful validator test.

---

[dafnamatsry]

Reviewed 4 of 5 files at r12, 5 of 5 files at r13, all commit messages.
Reviewable status: 9 of 10 files reviewed, all discussions resolved (waiting on @ayeletstarkware and @MohammadNassar1)

[ArniStarkware]

crates/gateway/src/gateway_test.rs line 86 at r13 (raw file):

    let response_bytes = &to_bytes(response).await;

    assert_eq!(status_code, StatusCode::OK, "{response_bytes:?}");

The base changed before this PR in #837

Code quote:

    let response = add_tx(State(app_state), rpc_tx.into()).await.into_response();

    let status_code = response.status();
    let response_bytes = &to_bytes(response).await;

    assert_eq!(status_code, StatusCode::OK, "{response_bytes:?}");

[dafnamatsry]

Reviewed 1 of 7 files at r14, 1 of 6 files at r16, 5 of 5 files at r17, all commit messages.
Reviewable status: 9 of 10 files reviewed, all discussions resolved (waiting on @ayeletstarkware and @MohammadNassar1)

[dafnamatsry]

Reviewable status: 9 of 10 files reviewed, all discussions resolved (waiting on @ayeletstarkware and @MohammadNassar1)

[ArniStarkware]

Reviewed 1 of 8 files at r8.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ayeletstarkware and @MohammadNassar1)