From 484c7fcbee3a2289138bbd54c9a199fd410e03a5 Mon Sep 17 00:00:00 2001 From: Arni Hod Date: Thu, 11 Apr 2024 13:25:04 +0300 Subject: [PATCH] test: generalize resource bounds related tests --- crates/gateway/src/starknet_api_test_utils.rs | 48 +++++---------- .../src/stateless_transaction_validator.rs | 60 +++++++++---------- .../stateless_transaction_validator_test.rs | 60 +++++++++++-------- 3 files changed, 79 insertions(+), 89 deletions(-) diff --git a/crates/gateway/src/starknet_api_test_utils.rs b/crates/gateway/src/starknet_api_test_utils.rs index 6d1975ad..80890e45 100644 --- a/crates/gateway/src/starknet_api_test_utils.rs +++ b/crates/gateway/src/starknet_api_test_utils.rs @@ -63,50 +63,32 @@ pub fn create_external_invoke_tx_for_testing( })) } -pub fn zero_resource_bounds_mapping() -> ResourceBoundsMapping { +pub const NON_EMPTY_RESOURCE_BOUNDS: ResourceBounds = ResourceBounds { + max_amount: 1, + max_price_per_unit: 1, +}; + +pub fn create_resource_bounds_mapping( + l1_resource_bounds: ResourceBounds, + l2_resource_bounds: ResourceBounds, +) -> ResourceBoundsMapping { ResourceBoundsMapping::try_from(vec![ ( starknet_api::transaction::Resource::L1Gas, - ResourceBounds::default(), + l1_resource_bounds, ), ( starknet_api::transaction::Resource::L2Gas, - ResourceBounds::default(), + l2_resource_bounds, ), ]) .expect("Resource bounds mapping has unexpected structure.") } -pub fn non_zero_l1_resource_bounds_mapping() -> ResourceBoundsMapping { - ResourceBoundsMapping::try_from(vec![ - ( - starknet_api::transaction::Resource::L1Gas, - ResourceBounds { - max_amount: 1, - max_price_per_unit: 1, - }, - ), - ( - starknet_api::transaction::Resource::L2Gas, - ResourceBounds::default(), - ), - ]) - .expect("Resource bounds mapping has unexpected structure.") +pub fn zero_resource_bounds_mapping() -> ResourceBoundsMapping { + create_resource_bounds_mapping(ResourceBounds::default(), ResourceBounds::default()) } -pub fn non_zero_l2_resource_bounds_mapping() -> ResourceBoundsMapping { - ResourceBoundsMapping::try_from(vec![ - ( - starknet_api::transaction::Resource::L1Gas, - ResourceBounds::default(), - ), - ( - starknet_api::transaction::Resource::L2Gas, - ResourceBounds { - max_amount: 1, - max_price_per_unit: 1, - }, - ), - ]) - .expect("Resource bounds mapping has unexpected structure.") +pub fn non_zero_resource_bounds_mapping() -> ResourceBoundsMapping { + create_resource_bounds_mapping(NON_EMPTY_RESOURCE_BOUNDS, NON_EMPTY_RESOURCE_BOUNDS) } diff --git a/crates/gateway/src/stateless_transaction_validator.rs b/crates/gateway/src/stateless_transaction_validator.rs index c2c86aa3..a3ada326 100644 --- a/crates/gateway/src/stateless_transaction_validator.rs +++ b/crates/gateway/src/stateless_transaction_validator.rs @@ -2,7 +2,7 @@ use starknet_api::external_transaction::{ ExternalDeclareTransaction, ExternalDeployAccountTransaction, ExternalInvokeTransaction, ExternalTransaction, }; -use starknet_api::transaction::Resource; +use starknet_api::transaction::{Resource, ResourceBoundsMapping}; use crate::errors::{TransactionValidatorError, TransactionValidatorResult}; @@ -12,7 +12,7 @@ mod transaction_validator_test; #[derive(Default)] pub struct StatelessTransactionValidatorConfig { - // If true, validates that the reousrce bounds are not zero. + // 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, } @@ -34,42 +34,40 @@ impl StatelessTransactionValidator { fn validate_fee(&self, tx: &ExternalTransaction) -> TransactionValidatorResult<()> { let resource_bounds_mapping = match tx { - ExternalTransaction::Declare(tx) => match tx { - ExternalDeclareTransaction::V3(tx) => &tx.resource_bounds, - }, - ExternalTransaction::DeployAccount(tx) => match tx { - ExternalDeployAccountTransaction::V3(tx) => &tx.resource_bounds, - }, - ExternalTransaction::Invoke(tx) => match tx { - ExternalInvokeTransaction::V3(tx) => &tx.resource_bounds, - }, - }; - - fn validate_reousrce_bounds( - resource_bounds_mapping: &starknet_api::transaction::ResourceBoundsMapping, - resource: Resource, - ) -> TransactionValidatorResult<()> { - if let Some(resource_bounds) = resource_bounds_mapping.0.get(&resource) { - if resource_bounds.max_amount == 0 || resource_bounds.max_price_per_unit == 0 { - return Err(TransactionValidatorError::ZeroFee { - resource, - resource_bounds: *resource_bounds, - }); - } - } else { - return Err(TransactionValidatorError::MissingResource { resource }); + ExternalTransaction::Declare(ExternalDeclareTransaction::V3(tx)) => &tx.resource_bounds, + ExternalTransaction::DeployAccount(ExternalDeployAccountTransaction::V3(tx)) => { + &tx.resource_bounds } - - Ok(()) - } + ExternalTransaction::Invoke(ExternalInvokeTransaction::V3(tx)) => &tx.resource_bounds, + }; if self.config.validate_non_zero_l1_gas_fee { - validate_reousrce_bounds(resource_bounds_mapping, Resource::L1Gas)?; + validate_resource_bounds(resource_bounds_mapping, Resource::L1Gas)?; } if self.config.validate_non_zero_l2_gas_fee { - validate_reousrce_bounds(resource_bounds_mapping, Resource::L2Gas)?; + validate_resource_bounds(resource_bounds_mapping, Resource::L2Gas)?; } Ok(()) } } + +// Utilities. + +fn validate_resource_bounds( + resource_bounds_mapping: &ResourceBoundsMapping, + resource: Resource, +) -> TransactionValidatorResult<()> { + if let Some(resource_bounds) = resource_bounds_mapping.0.get(&resource) { + if resource_bounds.max_amount == 0 || resource_bounds.max_price_per_unit == 0 { + return Err(TransactionValidatorError::ZeroFee { + resource, + resource_bounds: *resource_bounds, + }); + } + } else { + return Err(TransactionValidatorError::MissingResource { resource }); + } + + Ok(()) +} diff --git a/crates/gateway/src/stateless_transaction_validator_test.rs b/crates/gateway/src/stateless_transaction_validator_test.rs index 5d09d299..5ac5324f 100644 --- a/crates/gateway/src/stateless_transaction_validator_test.rs +++ b/crates/gateway/src/stateless_transaction_validator_test.rs @@ -5,27 +5,22 @@ use starknet_api::transaction::{Resource, ResourceBounds, ResourceBoundsMapping} use crate::starknet_api_test_utils::{ create_external_declare_tx_for_testing, create_external_deploy_account_tx_for_testing, - create_external_invoke_tx_for_testing, non_zero_l1_resource_bounds_mapping, - non_zero_l2_resource_bounds_mapping, zero_resource_bounds_mapping, + create_external_invoke_tx_for_testing, create_resource_bounds_mapping, + non_zero_resource_bounds_mapping, zero_resource_bounds_mapping, NON_EMPTY_RESOURCE_BOUNDS, }; use crate::stateless_transaction_validator::{ StatelessTransactionValidator, StatelessTransactionValidatorConfig, TransactionValidatorError, TransactionValidatorResult, }; -const VALIDATOR_CONFIG_FOR_TESTING: StatelessTransactionValidatorConfig = +const DEFAULT_VALIDATOR_CONFIG_FOR_TESTING: StatelessTransactionValidatorConfig = StatelessTransactionValidatorConfig { validate_non_zero_l1_gas_fee: true, - validate_non_zero_l2_gas_fee: false, - }; - -const VALIDATOR_CONFIG_FOR_L2_GAS_TESTING: StatelessTransactionValidatorConfig = - StatelessTransactionValidatorConfig { - validate_non_zero_l1_gas_fee: false, validate_non_zero_l2_gas_fee: true, }; #[rstest] +// Resource bounds validation tests. #[case::ignore_resource_bounds( StatelessTransactionValidatorConfig{ validate_non_zero_l1_gas_fee: false, @@ -35,47 +30,62 @@ const VALIDATOR_CONFIG_FOR_L2_GAS_TESTING: StatelessTransactionValidatorConfig = Ok(()) )] #[case::missing_l1_gas_resource_bounds( - VALIDATOR_CONFIG_FOR_TESTING, + StatelessTransactionValidatorConfig { + validate_non_zero_l1_gas_fee: true, + validate_non_zero_l2_gas_fee: false + }, create_external_invoke_tx_for_testing(ResourceBoundsMapping::default()), Err(TransactionValidatorError::MissingResource { resource: Resource::L1Gas }) )] #[case::missing_l2_gas_resource_bounds( - VALIDATOR_CONFIG_FOR_L2_GAS_TESTING, + StatelessTransactionValidatorConfig { + validate_non_zero_l1_gas_fee: false, + validate_non_zero_l2_gas_fee: true + }, create_external_invoke_tx_for_testing(ResourceBoundsMapping::default()), Err(TransactionValidatorError::MissingResource { resource: Resource::L2Gas }) )] #[case::zero_l1_gas_resource_bounds( - VALIDATOR_CONFIG_FOR_TESTING, + DEFAULT_VALIDATOR_CONFIG_FOR_TESTING, create_external_invoke_tx_for_testing(zero_resource_bounds_mapping()), + Err(TransactionValidatorError::ZeroFee{ resource: Resource::L1Gas, resource_bounds: ResourceBounds::default() }) )] #[case::zero_l2_gas_resource_bounds( - VALIDATOR_CONFIG_FOR_L2_GAS_TESTING, - create_external_invoke_tx_for_testing(non_zero_l1_resource_bounds_mapping()), + DEFAULT_VALIDATOR_CONFIG_FOR_TESTING, + create_external_invoke_tx_for_testing( + create_resource_bounds_mapping(NON_EMPTY_RESOURCE_BOUNDS, ResourceBounds::default()) + ), Err(TransactionValidatorError::ZeroFee{ resource: Resource::L2Gas, resource_bounds: ResourceBounds::default() }) )] +#[case::valid_l2_gas_invoke_tx( + StatelessTransactionValidatorConfig { + validate_non_zero_l1_gas_fee: false, + validate_non_zero_l2_gas_fee: true, + }, + create_external_invoke_tx_for_testing( + create_resource_bounds_mapping(ResourceBounds::default(), NON_EMPTY_RESOURCE_BOUNDS) + ), + Ok(()) +)] +// General flow. #[case::valid_declare_tx( - VALIDATOR_CONFIG_FOR_TESTING, - create_external_declare_tx_for_testing(non_zero_l1_resource_bounds_mapping()), + DEFAULT_VALIDATOR_CONFIG_FOR_TESTING, + create_external_declare_tx_for_testing(non_zero_resource_bounds_mapping()), Ok(()) )] #[case::valid_deploy_account_tx( - VALIDATOR_CONFIG_FOR_TESTING, - create_external_deploy_account_tx_for_testing(non_zero_l1_resource_bounds_mapping(),), + DEFAULT_VALIDATOR_CONFIG_FOR_TESTING, + create_external_deploy_account_tx_for_testing(non_zero_resource_bounds_mapping(),), Ok(()) )] #[case::valid_invoke_tx( - VALIDATOR_CONFIG_FOR_TESTING, - create_external_invoke_tx_for_testing(non_zero_l1_resource_bounds_mapping()), - Ok(()) -)] -#[case::valid_l2_gas_invoke_tx( - VALIDATOR_CONFIG_FOR_L2_GAS_TESTING, - create_external_invoke_tx_for_testing(non_zero_l2_resource_bounds_mapping()), + DEFAULT_VALIDATOR_CONFIG_FOR_TESTING, + create_external_invoke_tx_for_testing(non_zero_resource_bounds_mapping()), Ok(()) )] fn test_transaction_validator(