From 16a7af139268dd0d1cc97329c9a9f99f134f2285 Mon Sep 17 00:00:00 2001 From: Arni Hod Date: Thu, 4 Apr 2024 11:10:29 +0300 Subject: [PATCH] feat: stateless validator add calldata len validator --- crates/gateway/src/errors.rs | 8 +++ crates/gateway/src/starknet_api_test_utils.rs | 8 ++- .../src/stateless_transaction_validator.rs | 40 +++++++++++- .../stateless_transaction_validator_test.rs | 62 ++++++++++++++----- 4 files changed, 100 insertions(+), 18 deletions(-) diff --git a/crates/gateway/src/errors.rs b/crates/gateway/src/errors.rs index 8752fa02..10759bef 100644 --- a/crates/gateway/src/errors.rs +++ b/crates/gateway/src/errors.rs @@ -24,6 +24,14 @@ pub enum TransactionValidatorError { }, #[error("The resource bounds mapping is missing a resource {resource:?}.")] MissingResource { resource: Resource }, + #[error( + "Calldata length exceeded maximum: length {calldata_length} + (allowed length: {max_calldata_length})." + )] + CalldataTooLong { + calldata_length: usize, + max_calldata_length: usize, + }, } pub type TransactionValidatorResult = Result; diff --git a/crates/gateway/src/starknet_api_test_utils.rs b/crates/gateway/src/starknet_api_test_utils.rs index 80890e45..dcd8bb37 100644 --- a/crates/gateway/src/starknet_api_test_utils.rs +++ b/crates/gateway/src/starknet_api_test_utils.rs @@ -4,7 +4,7 @@ use starknet_api::external_transaction::{ ExternalDeployAccountTransactionV3, ExternalInvokeTransaction, ExternalInvokeTransactionV3, ExternalTransaction, }; -use starknet_api::transaction::{ResourceBounds, ResourceBoundsMapping}; +use starknet_api::transaction::{Calldata, ResourceBounds, ResourceBoundsMapping}; // Utils. pub fn create_external_declare_tx_for_testing( @@ -29,6 +29,7 @@ pub fn create_external_declare_tx_for_testing( pub fn create_external_deploy_account_tx_for_testing( resource_bounds: ResourceBoundsMapping, + constructor_calldata: Calldata, ) -> ExternalTransaction { ExternalTransaction::DeployAccount(ExternalDeployAccountTransaction::V3( ExternalDeployAccountTransactionV3 { @@ -36,7 +37,7 @@ pub fn create_external_deploy_account_tx_for_testing( tip: Default::default(), contract_address_salt: Default::default(), class_hash: Default::default(), - constructor_calldata: Default::default(), + constructor_calldata, nonce: Default::default(), signature: Default::default(), nonce_data_availability_mode: DataAvailabilityMode::L1, @@ -48,6 +49,7 @@ pub fn create_external_deploy_account_tx_for_testing( pub fn create_external_invoke_tx_for_testing( resource_bounds: ResourceBoundsMapping, + calldata: Calldata, ) -> ExternalTransaction { ExternalTransaction::Invoke(ExternalInvokeTransaction::V3(ExternalInvokeTransactionV3 { resource_bounds, @@ -55,7 +57,7 @@ pub fn create_external_invoke_tx_for_testing( signature: Default::default(), nonce: Default::default(), sender_address: Default::default(), - calldata: Default::default(), + calldata, nonce_data_availability_mode: DataAvailabilityMode::L1, fee_data_availability_mode: DataAvailabilityMode::L1, paymaster_data: Default::default(), diff --git a/crates/gateway/src/stateless_transaction_validator.rs b/crates/gateway/src/stateless_transaction_validator.rs index a3ada326..5ca5e6dc 100644 --- a/crates/gateway/src/stateless_transaction_validator.rs +++ b/crates/gateway/src/stateless_transaction_validator.rs @@ -15,6 +15,8 @@ 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 struct StatelessTransactionValidator { @@ -25,9 +27,9 @@ impl StatelessTransactionValidator { pub fn validate(&self, tx: &ExternalTransaction) -> TransactionValidatorResult<()> { // TODO(Arni, 1/5/2024): Add a mechanism that validate the sender address is not blocked. // TODO(Arni, 1/5/2024): Validate transaction version. - // TODO(Arni, 4/4/2024): Validate tx signature and calldata are not too long. self.validate_fee(tx)?; + self.validate_tx_size(tx)?; Ok(()) } @@ -50,6 +52,42 @@ impl StatelessTransactionValidator { Ok(()) } + + fn validate_tx_size(&self, tx: &ExternalTransaction) -> TransactionValidatorResult<()> { + self.validate_tx_calldata_size(tx)?; + + // TODO(Arni, 4/4/2024): Validate tx signature is not too long. + + Ok(()) + } + + fn validate_tx_calldata_size( + &self, + tx: &ExternalTransaction, + ) -> TransactionValidatorResult<()> { + let calldata = match tx { + ExternalTransaction::Declare(_) => { + // Declare transaction has no calldata. + return Ok(()); + } + ExternalTransaction::DeployAccount(tx) => match tx { + ExternalDeployAccountTransaction::V3(tx) => &tx.constructor_calldata, + }, + ExternalTransaction::Invoke(tx) => match tx { + ExternalInvokeTransaction::V3(tx) => &tx.calldata, + }, + }; + + let calldata_length = calldata.0.len(); + if calldata_length > self.config.max_calldata_length { + return Err(TransactionValidatorError::CalldataTooLong { + calldata_length, + max_calldata_length: self.config.max_calldata_length, + }); + } + + Ok(()) + } } // Utilities. diff --git a/crates/gateway/src/stateless_transaction_validator_test.rs b/crates/gateway/src/stateless_transaction_validator_test.rs index 5ac5324f..96f03e42 100644 --- a/crates/gateway/src/stateless_transaction_validator_test.rs +++ b/crates/gateway/src/stateless_transaction_validator_test.rs @@ -1,7 +1,9 @@ use rstest::rstest; +use starknet_api::calldata; use starknet_api::external_transaction::ExternalTransaction; -use starknet_api::transaction::{Resource, ResourceBounds, ResourceBoundsMapping}; +use starknet_api::hash::StarkFelt; +use starknet_api::transaction::{Calldata, Resource, ResourceBounds, ResourceBoundsMapping}; use crate::starknet_api_test_utils::{ create_external_declare_tx_for_testing, create_external_deploy_account_tx_for_testing, @@ -17,6 +19,8 @@ const DEFAULT_VALIDATOR_CONFIG_FOR_TESTING: StatelessTransactionValidatorConfig StatelessTransactionValidatorConfig { validate_non_zero_l1_gas_fee: true, validate_non_zero_l2_gas_fee: true, + + max_calldata_length: 1, }; #[rstest] @@ -25,30 +29,32 @@ const DEFAULT_VALIDATOR_CONFIG_FOR_TESTING: StatelessTransactionValidatorConfig StatelessTransactionValidatorConfig{ validate_non_zero_l1_gas_fee: false, validate_non_zero_l2_gas_fee: false, + ..DEFAULT_VALIDATOR_CONFIG_FOR_TESTING }, - create_external_invoke_tx_for_testing(zero_resource_bounds_mapping()), + create_external_invoke_tx_for_testing(zero_resource_bounds_mapping(), calldata![]), Ok(()) )] #[case::missing_l1_gas_resource_bounds( StatelessTransactionValidatorConfig { validate_non_zero_l1_gas_fee: true, - validate_non_zero_l2_gas_fee: false + validate_non_zero_l2_gas_fee: false, + ..DEFAULT_VALIDATOR_CONFIG_FOR_TESTING }, - create_external_invoke_tx_for_testing(ResourceBoundsMapping::default()), + create_external_invoke_tx_for_testing(ResourceBoundsMapping::default(), calldata![]), Err(TransactionValidatorError::MissingResource { resource: Resource::L1Gas }) )] #[case::missing_l2_gas_resource_bounds( StatelessTransactionValidatorConfig { validate_non_zero_l1_gas_fee: false, - validate_non_zero_l2_gas_fee: true + validate_non_zero_l2_gas_fee: true, + ..DEFAULT_VALIDATOR_CONFIG_FOR_TESTING }, - create_external_invoke_tx_for_testing(ResourceBoundsMapping::default()), + create_external_invoke_tx_for_testing(ResourceBoundsMapping::default(), calldata![]), Err(TransactionValidatorError::MissingResource { resource: Resource::L2Gas }) )] #[case::zero_l1_gas_resource_bounds( DEFAULT_VALIDATOR_CONFIG_FOR_TESTING, - create_external_invoke_tx_for_testing(zero_resource_bounds_mapping()), - + create_external_invoke_tx_for_testing(zero_resource_bounds_mapping(), calldata![]), Err(TransactionValidatorError::ZeroFee{ resource: Resource::L1Gas, resource_bounds: ResourceBounds::default() }) @@ -56,23 +62,51 @@ const DEFAULT_VALIDATOR_CONFIG_FOR_TESTING: StatelessTransactionValidatorConfig #[case::zero_l2_gas_resource_bounds( DEFAULT_VALIDATOR_CONFIG_FOR_TESTING, create_external_invoke_tx_for_testing( - create_resource_bounds_mapping(NON_EMPTY_RESOURCE_BOUNDS, ResourceBounds::default()) + create_resource_bounds_mapping(NON_EMPTY_RESOURCE_BOUNDS, ResourceBounds::default()), + calldata![] ), Err(TransactionValidatorError::ZeroFee{ resource: Resource::L2Gas, resource_bounds: ResourceBounds::default() }) )] #[case::valid_l2_gas_invoke_tx( - StatelessTransactionValidatorConfig { + StatelessTransactionValidatorConfig{ validate_non_zero_l1_gas_fee: false, validate_non_zero_l2_gas_fee: true, + ..DEFAULT_VALIDATOR_CONFIG_FOR_TESTING }, create_external_invoke_tx_for_testing( - create_resource_bounds_mapping(ResourceBounds::default(), NON_EMPTY_RESOURCE_BOUNDS) + create_resource_bounds_mapping(ResourceBounds::default(), NON_EMPTY_RESOURCE_BOUNDS), + calldata![] + ), + Ok(()) +)] +// Transaction size validation tests. +#[case::deploy_account_calldata_too_long( + DEFAULT_VALIDATOR_CONFIG_FOR_TESTING, + create_external_deploy_account_tx_for_testing( + non_zero_resource_bounds_mapping(), + calldata![StarkFelt::from_u128(1),StarkFelt::from_u128(2)], + ), + Err(TransactionValidatorError::CalldataTooLong { calldata_length: 2, max_calldata_length: 1 }) +)] +#[case::invoke_calldata_too_long( + DEFAULT_VALIDATOR_CONFIG_FOR_TESTING, + create_external_invoke_tx_for_testing( + non_zero_resource_bounds_mapping(), + calldata![StarkFelt::from_u128(1),StarkFelt::from_u128(2)], + ), + Err(TransactionValidatorError::CalldataTooLong { calldata_length: 2, max_calldata_length: 1 }) +)] +#[case::non_empty_valid_calldata( + DEFAULT_VALIDATOR_CONFIG_FOR_TESTING, + create_external_invoke_tx_for_testing( + non_zero_resource_bounds_mapping(), + calldata![StarkFelt::from_u128(1)], ), Ok(()) )] -// General flow. +// General cases. #[case::valid_declare_tx( DEFAULT_VALIDATOR_CONFIG_FOR_TESTING, create_external_declare_tx_for_testing(non_zero_resource_bounds_mapping()), @@ -80,12 +114,12 @@ const DEFAULT_VALIDATOR_CONFIG_FOR_TESTING: StatelessTransactionValidatorConfig )] #[case::valid_deploy_account_tx( DEFAULT_VALIDATOR_CONFIG_FOR_TESTING, - create_external_deploy_account_tx_for_testing(non_zero_resource_bounds_mapping(),), + create_external_deploy_account_tx_for_testing(non_zero_resource_bounds_mapping(), calldata![]), Ok(()) )] #[case::valid_invoke_tx( DEFAULT_VALIDATOR_CONFIG_FOR_TESTING, - create_external_invoke_tx_for_testing(non_zero_resource_bounds_mapping()), + create_external_invoke_tx_for_testing(non_zero_resource_bounds_mapping(), calldata![]), Ok(()) )] fn test_transaction_validator(