chore(blockifier): remove redundant blockifier transactions and related fns

[avivg-starkware]

…ed fns

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload triggered. View details here

[codecov]

Codecov Report

Attention: Patch coverage is 5.55556% with 17 lines in your changes missing coverage. Please review.

Project coverage is 77.14%. Comparing base (e3165c4) to head (74cf630).
Report is 364 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/starknet_api/src/executable_transaction.rs	0.00%	17 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1831       +/-   ##
===========================================
+ Coverage   40.10%   77.14%   +37.04%     
===========================================
  Files          26      103       +77     
  Lines        1895    13562    +11667     
  Branches     1895    13562    +11667     
===========================================
+ Hits          760    10463     +9703     
- Misses       1100     2644     +1544     
- Partials       35      455      +420     

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

[github-actions]

Artifacts upload triggered. View details here

[avivg-starkware]

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

---

crates/blockifier/src/execution/contract_class.rs line 525 at r1 (raw file):

#[derive(Clone, Debug)]
// TODO(Ayelet,10/02/2024): Change to bytes.

Copied this line to starknet_api::contract_class::ClassInfo (below)

[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]

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

---

crates/blockifier/src/execution/contract_class.rs line 47 at r3 (raw file):

pub mod test;

pub type ContractClassResult<T> = Result<T, ContractClassError>;

Delete this, and delete ContractClassError.

Code quote:

pub type ContractClassResult<T> = Result<T, ContractClassError>;

[ArniStarkware]

Reviewed 3 of 5 files at r1, 1 of 1 files at r3.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @avivg-starkware, @meship-starkware, and @noaov1)

---

crates/blockifier/src/transaction/transactions.rs line 163 at r3 (raw file):

        let declare_version = declare_tx.version();
        // Verify contract class version.
        // TODO(Noa): Avoid the unnecessary conversion.

Was the corresponding todo created somewhere else during these PRs?
If so - address it.

Code quote:

// TODO(Noa): Avoid the unnecessary conversion.

[github-actions]

Artifacts upload triggered. View details here

[meship-starkware]

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

[github-actions]

Artifacts upload triggered. View details here

[avivg-starkware]

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

---

crates/starknet_api/src/executable_transaction.rs line 213 at r9 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Both

Hope now it's good, if not I'll change in a small PR

[github-actions]

Artifacts upload triggered. View details here

[avivg-starkware]

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

---

crates/starknet_api/src/executable_transaction.rs line 245 at r13 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Add line.

Thankyou!

[ArniStarkware]

Reviewable status: 6 of 29 files reviewed, all discussions resolved (waiting on @meship-starkware and @noaov1)

---

crates/starknet_api/src/executable_transaction.rs line 245 at r13 (raw file):

Previously, avivg-starkware wrote…

Thankyou!

also after the block of the function validate_class_version_matches_tx_version and before the derives of DeployAccountTransaction

[ArniStarkware]

Reviewable status: 6 of 29 files reviewed, 1 unresolved discussion (waiting on @avivg-starkware, @meship-starkware, and @noaov1)

[meship-starkware]

Reviewed 23 of 24 files at r12, 1 of 1 files at r15, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @avivg-starkware and @noaov1)

[avivg-starkware]

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

---

crates/starknet_api/src/executable_transaction.rs line 245 at r13 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

also after the block of the function validate_class_version_matches_tx_version and before the derives of DeployAccountTransaction

Done.

[github-actions]

Artifacts upload triggered. View details here

[meship-starkware]

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

[meship-starkware]

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

[ArniStarkware]

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

[Yoni-Starkware]

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

---

crates/starknet_api/src/executable_transaction.rs line 216 at r16 (raw file):

/// Validates that the Declare transaction version is compatible with the Cairo contract version.
/// Versions 0 and 1 declare Cairo 0 contracts, while versions 2 and 3 declare Cairo 1 contracts.

Suggestion:

/// Versions 0 and 1 declare Cairo 0 contracts, while versions >= 2 declare Cairo 1 contracts.

---

crates/starknet_api/src/executable_transaction.rs line 217 at r16 (raw file):

/// Validates that the Declare transaction version is compatible with the Cairo contract version.
/// Versions 0 and 1 declare Cairo 0 contracts, while versions 2 and 3 declare Cairo 1 contracts.
fn validate_class_version_matches_tx_version(

For the next time: this refactor deserves a separate PR.

Code quote:

fn validate_class_version_matches_tx_version(

---

crates/starknet_api/src/executable_transaction.rs line 234 at r16 (raw file):

        ContractClass::V1(_) => {
            if !(declare_version == TransactionVersion::TWO
                || declare_version == TransactionVersion::THREE)

Why not <, >=?
Future versions are also going to hold Cairo1 contracts.

Code quote:

            if !(declare_version == TransactionVersion::ZERO
                || (declare_version == TransactionVersion::ONE))
            {
                Err(StarknetApiError::ContractClassVersionMismatch {
                    declare_version,
                    cairo_version: 0,
                })?
            }
        }
        ContractClass::V1(_) => {
            if !(declare_version == TransactionVersion::TWO
                || declare_version == TransactionVersion::THREE)

[Yoni-Starkware]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @avivg-starkware and @noaov1)

---

crates/blockifier/src/transaction/transactions.rs line 10 at r16 (raw file):

    DeployAccountTransaction as ExecutableDeployAccountTx,
    InvokeTransaction as ExecutableInvokeTx,
    L1HandlerTransaction,

No need for these nicknames anymore, right?

Code quote:

use starknet_api::executable_transaction::{
    DeclareTransaction as ExecutableDeclareTx,
    DeployAccountTransaction as ExecutableDeployAccountTx,
    InvokeTransaction as ExecutableInvokeTx,
    L1HandlerTransaction,

[github-actions]

Artifacts upload triggered. View details here

[ArniStarkware]

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

---

crates/starknet_api/src/executable_transaction.rs line 223 at r17 (raw file):

    let (expected_cairo_version, is_valid_version) = match class {
        ContractClass::V0(_) => (0, declare_version <= TransactionVersion::ONE),
        ContractClass::V1(_) => (1, declare_version > TransactionVersion::ONE),

This is more explicit.

Suggestion:

declare_version >= TransactionVersion::TWO

[github-actions]

Artifacts upload triggered. View details here

[avivg-starkware]

Reviewable status: 28 of 29 files reviewed, 5 unresolved discussions (waiting on @ArniStarkware, @meship-starkware, @noaov1, and @Yoni-Starkware)

---

crates/blockifier/src/transaction/transactions.rs line 10 at r16 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

No need for these nicknames anymore, right?

Yes indeed

---

crates/starknet_api/src/executable_transaction.rs line 217 at r16 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

For the next time: this refactor deserves a separate PR.

yes will do

---

crates/starknet_api/src/executable_transaction.rs line 234 at r16 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

Why not <, >=?
Future versions are also going to hold Cairo1 contracts.

How about now?

---

crates/starknet_api/src/executable_transaction.rs line 223 at r17 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

This is more explicit.

Sorry, I had to change, there was a problem with this implementation -
'expected_cairo_version' was incorrect in terms of the implementation of 'ContractClassVersionMismatch'.

how is it now?

(in more details-
ContractClassVersionMismatch expects to receive the Cairo version compatible w 'declare_version', in the case here, I passed on the cairo_version that is the input of the func (the 'wrong' cairo_version).

---

crates/starknet_api/src/executable_transaction.rs line 216 at r16 (raw file):

/// Validates that the Declare transaction version is compatible with the Cairo contract version.
/// Versions 0 and 1 declare Cairo 0 contracts, while versions 2 and 3 declare Cairo 1 contracts.

Done.

[Yoni-Starkware]

Reviewed 1 of 2 files at r17, 1 of 1 files at r18, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArniStarkware, @meship-starkware, and @noaov1)

---

crates/starknet_api/src/executable_transaction.rs line 234 at r16 (raw file):

Previously, avivg-starkware wrote…

How about now?

Cool :)

[ArniStarkware]

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

---

crates/starknet_api/src/executable_transaction.rs line 223 at r17 (raw file):

Previously, avivg-starkware wrote…

Sorry, I had to change, there was a problem with this implementation -
'expected_cairo_version' was incorrect in terms of the implementation of 'ContractClassVersionMismatch'.

how is it now?

(in more details-
ContractClassVersionMismatch expects to receive the Cairo version compatible w 'declare_version', in the case here, I passed on the cairo_version that is the input of the func (the 'wrong' cairo_version).

The new version is better. Nice.