feat: check that the compiled class hash matches the supplied class

[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 82.00%. Comparing base (8e9b2d7) to head (b12db17).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #246      +/-   ##
==========================================
- Coverage   83.21%   82.00%   -1.21%     
==========================================
  Files          28       28              
  Lines        1382     1423      +41     
  Branches     1382     1423      +41     
==========================================
+ Hits         1150     1167      +17     
- Misses        173      197      +24     
  Partials       59       59              

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

[ArniStarkware]

Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion

a discussion (no related file):
Add unit test for this part.

---

[ArniStarkware]

In this PR, we enforce the compiled class hash supplied by the transaction matchs the computed compiled class hash.

Reviewable status: 0 of 9 files reviewed, all discussions resolved

[Yael-Starkware]

Reviewed 1 of 6 files at r2, 8 of 8 files at r3.
Reviewable status: 8 of 9 files reviewed, 3 unresolved discussions (waiting on @ArniStarkware and @dafnamatsry)

---

crates/gateway/src/gateway_test.rs line 133 at r3 (raw file):

        _ => panic!("Invalid transaction type"),
    };
    let RPCDeclareTransaction::V3(declare_tx_v3) = &declare_tx;

Can you match the downcast code to the previous function?

Code quote:

    let declare_tx = match declare_tx() {
        RPCTransaction::Declare(declare_tx) => declare_tx,
        _ => panic!("Invalid transaction type"),
    };
    let RPCDeclareTransaction::V3(declare_tx_v3) = &declare_tx;

---

crates/gateway/src/gateway_test.rs line 141 at r3 (raw file):

        Ok(class_info)
        if (
            matches!(class_info.contract_class(), ContractClass::V1(_))

I guess there is no easy way to verify the result of the compilation,
but what do you check here? only the cairo version?

---

crates/gateway/src/gateway_test.rs line 144 at r3 (raw file):

            && class_info.sierra_program_length() == contract_class.sierra_program.len()
            && class_info.abi_length() == contract_class.abi.len()
        )

Can you also check that the supplied compiled class hash matches the computed compiled class hash?

[ArniStarkware]

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

---

crates/gateway/src/gateway_test.rs line 133 at r3 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

Can you match the downcast code to the previous function?

The difference is that here, we want to use declare_tx as the input of the function compile_contract_class.

It is more direct here. Otherwise, I will unwrap a RPCDeclareTransaction and then wrap the same value again.

If anything - I would change the previous test to get a mutable RPCDeclareTransaction - mutate it and use it in the test - but I am unsure how.

---

crates/gateway/src/gateway_test.rs line 141 at r3 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

I guess there is no easy way to verify the result of the compilation,
but what do you check here? only the cairo version?

First, we check that the compilation was successful. Other than that, yes, we check the Cairo version.

---

crates/gateway/src/gateway_test.rs line 144 at r3 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

Can you also check that the supplied compiled class hash matches the computed compiled class hash?

It is checked inside the function compile_contract_class. Testing it here is will just repeat this logic. This subject is being tested in test_compile_contract_class_failure.

[dafnamatsry]

Reviewed 4 of 8 files at r3, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @ArniStarkware and @Yael-Starkware)

---

crates/gateway/src/gateway_test.rs line 112 at r5 (raw file):

        RPCTransaction::Declare(RPCDeclareTransaction::V3(declare_tx)) => declare_tx,
        _ => panic!("Invalid transaction type"),
    };

Suggestion:

    let tx = assert_matches!(
        declare_tx(),
        RPCTransaction::Declare(RPCDeclareTransaction::V3(tx)) => tx
    );
 

---

crates/gateway/src/gateway_test.rs line 132 at r5 (raw file):

        RPCTransaction::Declare(declare_tx) => declare_tx,
        _ => panic!("Invalid transaction type"),
    };

Same as above.

Code quote:

    let declare_tx = match declare_tx() {
        RPCTransaction::Declare(declare_tx) => declare_tx,
        _ => panic!("Invalid transaction type"),
    };

---

crates/gateway/src/gateway_test.rs line 144 at r5 (raw file):

            && class_info.sierra_program_length() == contract_class.sierra_program.len()
            && class_info.abi_length() == contract_class.abi.len()
        )

Please separate to 3 different assert statements.

Code quote:

    assert_matches!(
        result,
        Ok(class_info)
        if (
            matches!(class_info.contract_class(), ContractClass::V1(_))
            && class_info.sierra_program_length() == contract_class.sierra_program.len()
            && class_info.abi_length() == contract_class.abi.len()
        )

---

crates/gateway/src/stateful_transaction_validator_test.rs line 49 at r5 (raw file):

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

Why did it change?

Code quote:

0x02da54b89e00d2e201f8e3ed2bcc715a69e89aefdce88aff2d2facb8dec55c0a

[ArniStarkware]

Reviewable status: 7 of 9 files reviewed, 7 unresolved discussions (waiting on @dafnamatsry and @Yael-Starkware)

---

crates/gateway/src/gateway_test.rs line 112 at r5 (raw file):

        RPCTransaction::Declare(RPCDeclareTransaction::V3(declare_tx)) => declare_tx,
        _ => panic!("Invalid transaction type"),
    };

Done.

---

crates/gateway/src/gateway_test.rs line 132 at r5 (raw file):

Previously, dafnamatsry wrote…

Same as above.

Done.

---

crates/gateway/src/gateway_test.rs line 144 at r5 (raw file):

Previously, dafnamatsry wrote…

Please separate to 3 different assert statements.

Done.

---

crates/gateway/src/stateful_transaction_validator_test.rs line 49 at r5 (raw file):

Previously, dafnamatsry wrote…

Why did it change?

compiled_class_hash changed from CompiledClassHash::Default().

[dafnamatsry]

Reviewed 1 of 6 files at r2, 3 of 8 files at r3, 2 of 2 files at r6, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ArniStarkware and @Yael-Starkware)

---

crates/gateway/src/gateway_test.rs line 165 at r6 (raw file):

        result,
        Ok(class_info) => class_info
    );

Suggestion:

    let class_info = compile_contract_class(&declare_tx).unwrap();

[ArniStarkware]

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

---

crates/gateway/src/gateway_test.rs line 165 at r6 (raw file):

        result,
        Ok(class_info) => class_info
    );

Done.

[dafnamatsry]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Yael-Starkware)

[Yael-Starkware]

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