feat: add declare specific size related stateless tx validator

[ArniStarkware]

This change is 

[ArniStarkware]

• feat: use config for stateless validator config sierra version #244
• feat: add declare specific stateless tx validations #226
• feat: add declare specific size related stateless tx validator #225 👈
• 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 75.73%. Comparing base (c94d1d7) to head (96e05b2).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #225      +/-   ##
==========================================
+ Coverage   74.78%   75.73%   +0.95%     
==========================================
  Files          23       23              
  Lines         916      952      +36     
  Branches      916      952      +36     
==========================================
+ Hits          685      721      +36     
  Misses        187      187              
  Partials       44       44              

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

[ArniStarkware]

Reviewable status: 0 of 4 files reviewed, all discussions resolved (waiting on @Yael-Starkware)

---

crates/gateway/src/stateless_transaction_validator.rs line 120 at r1 (raw file):

        let bytecode_size = contract_class.sierra_program.len();
        let serialized_class = serde_json::to_string(&contract_class)
            .expect("Unexpected error serializing contract class.");

I use expect here as I do not understand how the serialization may fail.

Should I use some safer mechanism?

Code quote:

        let serialized_class = serde_json::to_string(&contract_class)
            .expect("Unexpected error serializing contract class.");

---

crates/gateway/src/stateless_transaction_validator.rs line 141 at r1 (raw file):

    raw_class_size: usize,
    max_raw_class_size: usize,
) -> Result<(), DeclareTransactionError> {

We expect to use this function for validations after the compilation as well. We will move it when time comes.

Code quote:

fn validate_class_size(
    bytecode_language: String,
    bytecode_size: usize,
    max_bytecode_size: usize,
    raw_class_size: usize,
    max_raw_class_size: usize,
) -> Result<(), DeclareTransactionError> {

[Yael-Starkware]

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ArniStarkware)

---

crates/gateway/src/stateless_transaction_validator.rs line 120 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

I use expect here as I do not understand how the serialization may fail.

Should I use some safer mechanism?

Not sure what can be a failure reason, one thing I found online is a limit on recursion depth. Not sure if this is relevant for this case but I would prefer the safe error handling here.

---

crates/gateway/src/stateless_transaction_validator.rs line 141 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

We expect to use this function for validations after the compilation as well. We will move it when time comes.

why check the same thing twice?

---

crates/gateway/src/stateless_transaction_validator_test.rs line 28 at r1 (raw file):

        max_signature_length: 1,
        max_bytecode_size: 3,
        max_raw_class_size: 408,

does these numbers have a meaning or are they random?

---

crates/gateway/src/stateless_transaction_validator_test.rs line 174 at r1 (raw file):

    };
    let mut sierra_program = vec![stark_felt!(1_u128), stark_felt!(3_u128), stark_felt!(0_u128)];
    sierra_program.extend(vec![stark_felt!(12_u128); config_max_bytecode_size]);

Suggestion:

    let sierra_program = vec![stark_felt!(1_u128); onfig_max_bytecode_size+3];

---

crates/gateway/src/stateless_transaction_validator_test.rs line 191 at r1 (raw file):

            assert_eq!(bytecode_size, sierra_program_length);
            assert_eq!(max_bytecode_size, config_max_bytecode_size);
        }

nice! I wasn't aware of this syntax

Code quote:

        ) => {
            assert_eq!(bytecode_language, "Sierra");
            assert_eq!(bytecode_size, sierra_program_length);
            assert_eq!(max_bytecode_size, config_max_bytecode_size);
        }

[ArniStarkware]

Reviewable status: 1 of 4 files reviewed, 2 unresolved discussions (waiting on @Yael-Starkware)

---

crates/gateway/src/stateless_transaction_validator.rs line 120 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

Not sure what can be a failure reason, one thing I found online is a limit on recursion depth. Not sure if this is relevant for this case but I would prefer the safe error handling here.

Done.

---

crates/gateway/src/stateless_transaction_validator.rs line 141 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

why check the same thing twice?

On the other case - it will validate the Casm class size. In this PR this function is used to validate the Sierra class size.

---

crates/gateway/src/stateless_transaction_validator_test.rs line 28 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

does these numbers have a meaning or are they random?

These are the minimal numbers for irrelevant cases to pass.
Fixed.

---

crates/gateway/src/stateless_transaction_validator_test.rs line 174 at r1 (raw file):

    };
    let mut sierra_program = vec![stark_felt!(1_u128), stark_felt!(3_u128), stark_felt!(0_u128)];
    sierra_program.extend(vec![stark_felt!(12_u128); config_max_bytecode_size]);

Done.

[Yael-Starkware]

Reviewed 2 of 3 files at r2, all commit messages.
Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @ArniStarkware)

---

crates/gateway/src/stateless_transaction_validator.rs line 120 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Done.

would be prettier to use "?" instead of map_err.

---

crates/gateway/src/stateless_transaction_validator_test.rs line 28 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

These are the minimal numbers for irrelevant cases to pass.
Fixed.

not sure I understand, these number are not test specific?
I'm asking because in test_declare_contract_class_size_too_long you have 3 felts in the sierra program , which equals the default max_bytecode_size that you defined here.
If these are test specific I think they shouldn't be in the default setting, but if they are general to any test then it's fine.

[ArniStarkware]

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

---

crates/gateway/src/errors.rs line 57 at r2 (raw file):

#[derive(Debug, Error)]
#[cfg_attr(test, derive(PartialEq))]
pub enum DeclareTransactionError {

@dafnamatsry WDYT?

Suggestion:

StatelessDeclareTransactionValidatorError

---

crates/gateway/src/starknet_api_test_utils.rs line 55 at r3 (raw file):

            };
            external_declare_tx(declare_tx_args!(resource_bounds, signature, contract_class))
        }

Used in the positive flow test.

Code quote:

        TransactionType::Declare => {
            let contract_class = ContractClass {
                sierra_program: vec![stark_felt!(1_u32); 3],
                ..ContractClass::default()
            };
            external_declare_tx(declare_tx_args!(resource_bounds, signature, contract_class))
        }

---

crates/gateway/src/stateless_transaction_validator.rs line 120 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

would be prettier to use "?" instead of map_err.

This means I need to have a transparent error.
I don't want StatelessTransactionValidatorError to expose the inner serde json error.

Besides - serde_json::Error does not implement PartialEq.

But of you insist once more - I will make it happen.

---

crates/gateway/src/stateless_transaction_validator_test.rs line 28 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

not sure I understand, these number are not test specific?
I'm asking because in test_declare_contract_class_size_too_long you have 3 felts in the sierra program , which equals the default max_bytecode_size that you defined here.
If these are test specific I think they shouldn't be in the default setting, but if they are general to any test then it's fine.

Replaced with non-test-specific values - but with a healthy margin.

Tests unrelated to these values should not fail on these values.

[Yael-Starkware]

Reviewable status: 1 of 5 files reviewed, all discussions resolved (waiting on @dafnamatsry)

[dafnamatsry]

Reviewed 1 of 4 files at r1, 1 of 3 files at r2, 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @ArniStarkware)

---

crates/gateway/src/config.rs line 75 at r3 (raw file):

    pub max_bytecode_size: usize,
    pub max_raw_class_size: usize,
}

Suggestion:

pub struct StatelessTransactionValidatorConfig {
    // If true, validates that the resource bounds are not zero.
    pub validate_non_zero_l1_gas_fee: bool,
    pub validate_non_zero_l2_gas_fee: bool,

    pub max_calldata_length: usize,
    pub max_signature_length: usize,
    
    // Declare txs specific config.
    pub max_bytecode_size: usize,
    pub max_raw_class_size: usize,
}

---

crates/gateway/src/errors.rs line 57 at r2 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

@dafnamatsry WDYT?

I don't think we need a specific type of Declare txs error. It can be under the StatelessTransactionValidatorError.
It complicates the code, and it is also confusing because it doesn't really include all the possible error (declare txs can also have errors that are general to all tx types).

---

crates/gateway/src/errors.rs line 62 at r3 (raw file):

        "Declared contract class {bytecode_language} bytecode size is {bytecode_size}. It must be \
         less then {max_bytecode_size}."
    )]

Please use the same string that we have now in the pythonic GW.

Code quote:

    #[error(
        "Declared contract class {bytecode_language} bytecode size is {bytecode_size}. It must be \
         less then {max_bytecode_size}."
    )]

---

crates/gateway/src/errors.rs line 71 at r3 (raw file):

        "Declared contract class {bytecode_language} size is {contract_class_object_size}. It \
         must be less then {max_contract_class_object_size}."
    )]

Same.

Code quote:

    #[error(
        "Declared contract class {bytecode_language} size is {contract_class_object_size}. It \
         must be less then {max_contract_class_object_size}."
    )]

---

crates/gateway/src/stateless_transaction_validator.rs line 120 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

This means I need to have a transparent error.
I don't want StatelessTransactionValidatorError to expose the inner serde json error.

Besides - serde_json::Error does not implement PartialEq.

But of you insist once more - I will make it happen.

I actually think that expect is more suitable here, since it is a bug of ours if it fails here.

---

crates/gateway/src/stateless_transaction_validator.rs line 141 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

On the other case - it will validate the Casm class size. In this PR this function is used to validate the Sierra class size.

In the Casm class validation we also have bytecode and raw sizes?
In any case, I would prefer to have a dedicated validate_casm_contract_class as it may have additional specific validations to Casm.

---

crates/gateway/src/stateless_transaction_validator.rs line 30 at r3 (raw file):

        self.validate_tx_size(tx)?;

        if let ExternalTransaction::Declare(ExternalDeclareTransaction::V3(declare_tx)) = tx {

Suggestion:

if let ExternalTransaction::Declare(declare_tx)

---

crates/gateway/src/stateless_transaction_validator.rs line 107 at r3 (raw file):

    fn validate_declare_tx(
        &self,
        tx: &ExternalDeclareTransactionV3,

Suggestion:

ExternalDeclareTransaction

---

crates/gateway/src/stateless_transaction_validator.rs line 111 at r3 (raw file):

        self.validate_class_length(&tx.contract_class)?;

        Ok(())

Suggestion:

        self.validate_class_length(&tx.contract_class)

---

crates/gateway/src/stateless_transaction_validator.rs line 132 at r3 (raw file):

            raw_class_size,
            self.config.max_raw_class_size,
        )?)

Please move the checks here and delete validate_class_size.

Code quote:

        Ok(validate_class_size(
            "Sierra".to_string(),
            bytecode_size,
            self.config.max_bytecode_size,
            raw_class_size,
            self.config.max_raw_class_size,
        )?)

---

crates/gateway/src/stateless_transaction_validator_test.rs line 25 at r3 (raw file):

        validate_non_zero_l2_gas_fee: true,

        max_calldata_length: 1,

Suggestion:

        validate_non_zero_l2_gas_fee: true,
        max_calldata_length: 1,

---

crates/gateway/src/stateless_transaction_validator_test.rs line 168 at r3 (raw file):

        config: StatelessTransactionValidatorConfig {
            validate_non_zero_l1_gas_fee: false,
            validate_non_zero_l2_gas_fee: false,

Why not use the default?

Code quote:

            validate_non_zero_l1_gas_fee: false,
            validate_non_zero_l2_gas_fee: false,

---

crates/gateway/src/stateless_transaction_validator_test.rs line 174 at r3 (raw file):

    };
    let sierra_program = vec![stark_felt!(1_u128); config_max_bytecode_size + 1];
    let sierra_program_length = sierra_program.len();

Suggestion:

    let sierra_program_length = config_max_bytecode_size + 1;
    let sierra_program = vec![stark_felt!(1_u128); sierra_program_length];
    

---

crates/gateway/src/stateless_transaction_validator_test.rs line 191 at r3 (raw file):

            assert_eq!(max_bytecode_size, config_max_bytecode_size);
        }
    )

Suggestion:

    assert_matches!(
        tx_validator.validate(&tx).unwrap_err(),
        StatelessTransactionValidatorError::DeclareTransactionError(
            DeclareTransactionError::BytecodeSizeTooLarge {
                bytecode_language,
                bytecode_size,
                max_bytecode_size
            }
        ) if (bytecode_language, bytecode_size, max_bytecode_size) ==
        ("Sierra", sierra_program_length, config_max_bytecode_size);
    )

---

crates/gateway/src/stateless_transaction_validator_test.rs line 200 at r3 (raw file):

        config: StatelessTransactionValidatorConfig {
            validate_non_zero_l1_gas_fee: false,
            validate_non_zero_l2_gas_fee: false,

Same.

Code quote:

            validate_non_zero_l1_gas_fee: false,
            validate_non_zero_l2_gas_fee: false,

---

crates/gateway/src/stateless_transaction_validator_test.rs line 207 at r3 (raw file):

    let contract_class =
        ContractClass { sierra_program: vec![stark_felt!(1_u128); 3], ..Default::default() };
    let contract_class_len = serde_json::to_string(&contract_class).unwrap().len();

Suggestion:

contract_class_length

---

crates/gateway/src/stateless_transaction_validator_test.rs line 219 at r3 (raw file):

            assert_eq!(max_contract_class_object_size, config_max_raw_class_size);
        }
    )

Same.

Code quote:

    assert_matches!(
        tx_validator.validate(&tx).unwrap_err(),
        StatelessTransactionValidatorError::DeclareTransactionError(
            DeclareTransactionError::ContractClassObjectSizeTooLarge { bytecode_language, contract_class_object_size, max_contract_class_object_size }
        ) => {
            assert_eq!(bytecode_language, "Sierra");
            assert_eq!(contract_class_object_size, contract_class_len);
            assert_eq!(max_contract_class_object_size, config_max_raw_class_size);
        }
    )

[dafnamatsry]

Reviewable status: 4 of 5 files reviewed, 15 unresolved discussions (waiting on @ArniStarkware)

---

crates/gateway/src/errors.rs line 51 at r4 (raw file):

    #[error(transparent)]
    DeclareTransactionError(#[from] DeclareTransactionError),
    // TODO(Arni): Cosider a transparant error for serde_json::Error.

Please remove.

Code quote:

// TODO(Arni): Cosider a transparant error for serde_json::Error.

[ArniStarkware]

Reviewable status: 0 of 5 files reviewed, 15 unresolved discussions (waiting on @dafnamatsry and @Yael-Starkware)

---

crates/gateway/src/config.rs line 75 at r3 (raw file):

    pub max_bytecode_size: usize,
    pub max_raw_class_size: usize,
}

Done.

---

crates/gateway/src/errors.rs line 57 at r2 (raw file):

Previously, dafnamatsry wrote…

I don't think we need a specific type of Declare txs error. It can be under the StatelessTransactionValidatorError.
It complicates the code, and it is also confusing because it doesn't really include all the possible error (declare txs can also have errors that are general to all tx types).

Done.

---

crates/gateway/src/errors.rs line 62 at r3 (raw file):

Previously, dafnamatsry wrote…

Please use the same string that we have now in the pythonic GW.

Done.

---

crates/gateway/src/errors.rs line 71 at r3 (raw file):

Previously, dafnamatsry wrote…

Same.

Done.

---

crates/gateway/src/errors.rs line 51 at r4 (raw file):

Previously, dafnamatsry wrote…

Please remove.

Done.

---

crates/gateway/src/stateless_transaction_validator.rs line 120 at r1 (raw file):

Previously, dafnamatsry wrote…

I actually think that expect is more suitable here, since it is a bug of ours if it fails here.

Done.

---

crates/gateway/src/stateless_transaction_validator.rs line 141 at r1 (raw file):

Previously, dafnamatsry wrote…

In the Casm class validation we also have bytecode and raw sizes?
In any case, I would prefer to have a dedicated validate_casm_contract_class as it may have additional specific validations to Casm.

Done.

We will extract that to a function if need be.

---

crates/gateway/src/stateless_transaction_validator.rs line 30 at r3 (raw file):

        self.validate_tx_size(tx)?;

        if let ExternalTransaction::Declare(ExternalDeclareTransaction::V3(declare_tx)) = tx {

Done.

---

crates/gateway/src/stateless_transaction_validator.rs line 107 at r3 (raw file):

    fn validate_declare_tx(
        &self,
        tx: &ExternalDeclareTransactionV3,

Done.

---

crates/gateway/src/stateless_transaction_validator.rs line 111 at r3 (raw file):

        self.validate_class_length(&tx.contract_class)?;

        Ok(())

Done.

---

crates/gateway/src/stateless_transaction_validator.rs line 132 at r3 (raw file):

Previously, dafnamatsry wrote…

Please move the checks here and delete validate_class_size.

Done.

---

crates/gateway/src/stateless_transaction_validator_test.rs line 25 at r3 (raw file):

        validate_non_zero_l2_gas_fee: true,

        max_calldata_length: 1,

Done.

---

crates/gateway/src/stateless_transaction_validator_test.rs line 168 at r3 (raw file):

Previously, dafnamatsry wrote…

Why not use the default?

Done. (was just fixed in a different PR).

---

crates/gateway/src/stateless_transaction_validator_test.rs line 174 at r3 (raw file):

    };
    let sierra_program = vec![stark_felt!(1_u128); config_max_bytecode_size + 1];
    let sierra_program_length = sierra_program.len();

Done.

---

crates/gateway/src/stateless_transaction_validator_test.rs line 191 at r3 (raw file):

            assert_eq!(max_bytecode_size, config_max_bytecode_size);
        }
    )

Done.

---

crates/gateway/src/stateless_transaction_validator_test.rs line 200 at r3 (raw file):

Previously, dafnamatsry wrote…

Same.

Done.

---

crates/gateway/src/stateless_transaction_validator_test.rs line 207 at r3 (raw file):

    let contract_class =
        ContractClass { sierra_program: vec![stark_felt!(1_u128); 3], ..Default::default() };
    let contract_class_len = serde_json::to_string(&contract_class).unwrap().len();

Done.

---

crates/gateway/src/stateless_transaction_validator_test.rs line 219 at r3 (raw file):

Previously, dafnamatsry wrote…

Same.

Done.

[dafnamatsry]

Reviewed 5 of 5 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Yael-Starkware)

[dafnamatsry]

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

---

crates/gateway/src/errors.rs line 63 at r5 (raw file):

    },
    #[error("Error serializing object: {object_name}")]
    SerializationError { object_name: String },

Is it used?

Code quote:

SerializationError

[ArniStarkware]

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

---

crates/gateway/src/errors.rs line 63 at r5 (raw file):

Previously, dafnamatsry wrote…

Is it used?

No :). Removed.

[dafnamatsry]

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