test: add tests for add tx on declare tx

[ArniStarkware]

This change is 

[ArniStarkware]

• feat: check that the compiled class hash matches the supplied class #246 : 3 dependent PRs (#296 , #298 , #312 )
• test: add tests for add tx on declare tx #235 👈
• feat: compile sierra to casm in declare tx #234
• 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-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.21%. Comparing base (c1827fc) to head (a70795b).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #235      +/-   ##
==========================================
+ Coverage   76.98%   83.21%   +6.22%     
==========================================
  Files          28       28              
  Lines        1382     1382              
  Branches     1382     1382              
==========================================
+ Hits         1064     1150      +86     
+ Misses        265      173      -92     
- Partials       53       59       +6     

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

[Yael-Starkware]

Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion (waiting on @ArniStarkware and @dafnamatsry)

---

crates/gateway/src/starknet_api_test_utils.rs line 96 at r2 (raw file):

pub fn declare_tx() -> ExternalTransaction {
    env::set_current_dir(get_absolute_path(TEST_FILES_FOLDER)).expect("Couldn't set working dir.");
    let json_file_path = Path::new(DECLARE_V3_TX_FILE);

I don't think we should take the transaction from a json file, we should create it in the code as we do with invoke.

[Yael-Starkware]

Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion (waiting on @ArniStarkware and @dafnamatsry)

---

crates/gateway/src/starknet_api_test_utils.rs line 96 at r2 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

I don't think we should take the transaction from a json file, we should create it in the code as we do with invoke.

(and also as in deploy_account - PR https://reviewable.io/reviews/starkware-libs/mempool/239#-)

[ArniStarkware]

crates/gateway/src/starknet_api_test_utils.rs line 96 at r2 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

(and also as in deploy_account - PR https://reviewable.io/reviews/starkware-libs/mempool/239#-)

The difference between Invoke/DeployAccount and Declare is that the declare tx is much bigger. The Sierra program is long.

I agree - this is also bad for consistency.
I think the solution is to load from a json only the contract_class / only the sierra_program.
WDYT?

[Yael-Starkware]

Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion (waiting on @ArniStarkware and @dafnamatsry)

---

crates/gateway/src/starknet_api_test_utils.rs line 96 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

The difference between Invoke/DeployAccount and Declare is that the declare tx is much bigger. The Sierra program is long.

I agree - this is also bad for consistency.
I think the solution is to load from a json only the contract_class / only the sierra_program.
WDYT?

I agree.

[ArniStarkware]

Reviewable status: 0 of 7 files reviewed, 1 unresolved discussion (waiting on @dafnamatsry and @Yael-Starkware)

---

crates/gateway/src/starknet_api_test_utils.rs line 96 at r2 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

I agree.

Done.

[ArniStarkware]

Reviewable status: 1 of 8 files reviewed, all discussions resolved (waiting on @dafnamatsry and @Yael-Starkware)

---

crates/gateway/src/stateful_transaction_validator_test.rs line 28 at r7 (raw file):

    local_test_state_reader_factory(CairoVersion::Cairo1, false),
    Ok(TransactionHash(StarkFelt::try_from(
        "0x007d70505b4487a4e1c1a4b4e4342cb5aa9e73b86d031891170c45a57ad8b4e6"

why was this hash affected?

Code quote:

       "0x007d70505b4487a4e1c1a4b4e4342cb5aa9e73b86d031891170c45a57ad8b4e6"

[ArniStarkware]

Reviewable status: 1 of 8 files reviewed, all discussions resolved (waiting on @dafnamatsry and @Yael-Starkware)

---

crates/gateway/src/stateful_transaction_validator_test.rs line 28 at r7 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

why was this hash affected?

Because I changed VALID_L1_GAS_MAX_AMOUNT.

[Yael-Starkware]

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

[dafnamatsry]

Reviewed 3 of 7 files at r5, 5 of 5 files at r7, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ArniStarkware)

---

crates/gateway/src/gateway_test.rs line 74 at r7 (raw file):

    local_test_state_reader_factory_for_deploy_account(&tx)
)]
#[case::declare_tx(declare_tx(), local_test_state_reader_factory(CairoVersion::Cairo1, false))]

to be consistent with the other test cases.

Suggestion:

valid_declare_tx

---

crates/gateway/src/gateway_test.rs line 102 at r7 (raw file):

    assert_eq!(tx_hash, serde_json::from_slice(response_bytes).unwrap());
}

Please add a test for bad declare tx (bad sierra program).

---

crates/gateway/src/starknet_api_test_utils.rs line 102 at r7 (raw file):

    let cairo_version = CairoVersion::Cairo1;
    let account_contract = FeatureContract::AccountWithoutValidations(cairo_version);

Suggestion:

    let account_contract = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1);

[ArniStarkware]

Reviewable status: 6 of 8 files reviewed, 3 unresolved discussions (waiting on @dafnamatsry and @Yael-Starkware)

---

crates/gateway/src/gateway_test.rs line 74 at r7 (raw file):

Previously, dafnamatsry wrote…

to be consistent with the other test cases.

Done.

---

crates/gateway/src/gateway_test.rs line 102 at r7 (raw file):

Previously, dafnamatsry wrote…

Please add a test for bad declare tx (bad sierra program).

I think it is redundant, as there are unit tests for the compile util.

Anyway - Covered in #246.

---

crates/gateway/src/starknet_api_test_utils.rs line 102 at r7 (raw file):

    let cairo_version = CairoVersion::Cairo1;
    let account_contract = FeatureContract::AccountWithoutValidations(cairo_version);

Done.

[dafnamatsry]

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