feat: stateless validator add non zero fee check (#29)

Cargo.toml

@@ -27,6 +27,7 @@ pretty_assertions = "1.4.0"
 rstest = "0.17.0"
 serde = { version = "1.0.193", features = ["derive"] }
 serde_json = "1.0"
+# TODO(Arni, 1/5/2024): Use a fixed version once the StarkNet API is stable.
 starknet_api = { git = "https://github.com/starkware-libs/starknet-api.git", rev = "6b14aaf" }
 papyrus_config = "0.3.0"
 thiserror = "1.0"

crates/gateway/src/errors.rs

@@ -1,3 +1,5 @@
+use starknet_api::transaction::{Resource, ResourceBounds};
+
 use thiserror::Error;
 
 #[derive(Debug, Error)]
@@ -14,6 +16,14 @@ pub enum GatewayError {
 
 #[derive(Debug, Error)]
 #[cfg_attr(test, derive(PartialEq))]
-pub enum TransactionValidatorError {}
+pub enum TransactionValidatorError {
+    #[error("Expected a positive amount of {resource:?}. Got {resource_bounds:?}.")]
+    ZeroFee {
+        resource: Resource,
+        resource_bounds: ResourceBounds,
+    },
+    #[error("The resource bounds mapping is missing a resource {resource:?}.")]
+    MissingResource { resource: Resource },
+}
 
 pub type TransactionValidatorResult<T> = Result<T, TransactionValidatorError>;

crates/gateway/src/starknet_api_test_utils.rs

@@ -7,10 +7,12 @@ use starknet_api::external_transaction::{
 use starknet_api::transaction::{ResourceBounds, ResourceBoundsMapping};
 
 // Utils.
-pub fn create_external_declare_tx_for_testing() -> ExternalTransaction {
+pub fn create_external_declare_tx_for_testing(
+    resource_bounds: ResourceBoundsMapping,
+) -> ExternalTransaction {
     ExternalTransaction::Declare(ExternalDeclareTransaction::V3(
         ExternalDeclareTransactionV3 {
-            resource_bounds: zero_resource_bounds_mapping(),
+            resource_bounds,
             contract_class: Default::default(),
             tip: Default::default(),
             signature: Default::default(),
@@ -25,10 +27,12 @@ pub fn create_external_declare_tx_for_testing() -> ExternalTransaction {
     ))
 }
 
-pub fn create_external_deploy_account_tx_for_testing() -> ExternalTransaction {
+pub fn create_external_deploy_account_tx_for_testing(
+    resource_bounds: ResourceBoundsMapping,
+) -> ExternalTransaction {
     ExternalTransaction::DeployAccount(ExternalDeployAccountTransaction::V3(
         ExternalDeployAccountTransactionV3 {
-            resource_bounds: zero_resource_bounds_mapping(),
+            resource_bounds,
             tip: Default::default(),
             contract_address_salt: Default::default(),
             class_hash: Default::default(),
@@ -42,9 +46,11 @@ pub fn create_external_deploy_account_tx_for_testing() -> ExternalTransaction {
     ))
 }
 
-pub fn create_external_invoke_tx_for_testing() -> ExternalTransaction {
+pub fn create_external_invoke_tx_for_testing(
+    resource_bounds: ResourceBoundsMapping,
+) -> ExternalTransaction {
     ExternalTransaction::Invoke(ExternalInvokeTransaction::V3(ExternalInvokeTransactionV3 {
-        resource_bounds: zero_resource_bounds_mapping(),
+        resource_bounds,
         tip: Default::default(),
         signature: Default::default(),
         nonce: Default::default(),
@@ -70,3 +76,37 @@ pub fn zero_resource_bounds_mapping() -> ResourceBoundsMapping {
     ])
     .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 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.")
+}

crates/gateway/src/transaction_validator.rs

@@ -1,24 +1,75 @@
-use starknet_api::external_transaction::ExternalTransaction;
+use starknet_api::external_transaction::{
+    ExternalDeclareTransaction, ExternalDeployAccountTransaction, ExternalInvokeTransaction,
+    ExternalTransaction,
+};
+use starknet_api::transaction::Resource;
 
-use crate::errors::TransactionValidatorResult;
+use crate::errors::{TransactionValidatorError, TransactionValidatorResult};
 
 #[cfg(test)]
 #[path = "transaction_validator_test.rs"]
 mod transaction_validator_test;
 
-pub struct TransactionValidatorConfig {}
+#[derive(Default)]
+pub struct TransactionValidatorConfig {
+    // If true, validates that the reousrce bounds are not zero.
+    pub validate_non_zero_l1_gas_fee: bool,
+    pub validate_non_zero_l2_gas_fee: bool,
+}
 
 pub struct TransactionValidator {
     pub config: TransactionValidatorConfig,
 }
 
 impl TransactionValidator {
-    pub fn validate(&self, _tx: ExternalTransaction) -> TransactionValidatorResult<()> {
+    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 fee non zero.
         // TODO(Arni, 4/4/2024): Validate tx signature and calldata are not too long.
 
+        self.validate_fee(&tx)?;
+
+        Ok(())
+    }
+
+    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 });
+            }
+
+            Ok(())
+        }
+
+        if self.config.validate_non_zero_l1_gas_fee {
+            validate_reousrce_bounds(resource_bounds_mapping, Resource::L1Gas)?;
+        }
+        if self.config.validate_non_zero_l2_gas_fee {
+            validate_reousrce_bounds(resource_bounds_mapping, Resource::L2Gas)?;
+        }
+
         Ok(())
     }
 }

crates/gateway/src/transaction_validator_test.rs

@@ -1,31 +1,80 @@
 use rstest::rstest;
 
 use starknet_api::external_transaction::ExternalTransaction;
+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,
+    create_external_invoke_tx_for_testing, non_zero_l1_resource_bounds_mapping,
+    non_zero_l2_resource_bounds_mapping, zero_resource_bounds_mapping,
 };
 use crate::transaction_validator::{
-    TransactionValidator, TransactionValidatorConfig, TransactionValidatorResult,
+    TransactionValidator, TransactionValidatorConfig, TransactionValidatorError,
+    TransactionValidatorResult,
 };
 
-const VALIDATOR_CONFIG_FOR_TESTING: TransactionValidatorConfig = TransactionValidatorConfig {};
+const VALIDATOR_CONFIG_FOR_TESTING: TransactionValidatorConfig = TransactionValidatorConfig {
+    validate_non_zero_l1_gas_fee: true,
+    validate_non_zero_l2_gas_fee: false,
+};
+
+const VALIDATOR_CONFIG_FOR_L2_GAS_TESTING: TransactionValidatorConfig =
+    TransactionValidatorConfig {
+        validate_non_zero_l1_gas_fee: false,
+        validate_non_zero_l2_gas_fee: true,
+    };
 
 #[rstest]
+#[case::ignore_resource_bounds(
+    TransactionValidatorConfig{
+        validate_non_zero_l1_gas_fee: false,
+        validate_non_zero_l2_gas_fee: false,
+    },
+    create_external_invoke_tx_for_testing(zero_resource_bounds_mapping()),
+    Ok(())
+)]
+#[case::missing_l1_gas_resource_bounds(
+    VALIDATOR_CONFIG_FOR_TESTING,
+    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,
+    create_external_invoke_tx_for_testing(ResourceBoundsMapping::default()),
+    Err(TransactionValidatorError::MissingResource { resource: Resource::L2Gas })
+)]
+#[case::zero_l1_gas_resource_bounds(
+    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()),
+    Err(TransactionValidatorError::ZeroFee{
+        resource: Resource::L2Gas, resource_bounds: ResourceBounds::default()
+    })
+)]
 #[case::valid_declare_tx(
     VALIDATOR_CONFIG_FOR_TESTING,
-    create_external_declare_tx_for_testing(),
+    create_external_declare_tx_for_testing(non_zero_l1_resource_bounds_mapping()),
     Ok(())
 )]
 #[case::valid_deploy_account_tx(
     VALIDATOR_CONFIG_FOR_TESTING,
-    create_external_deploy_account_tx_for_testing(),
+    create_external_deploy_account_tx_for_testing(non_zero_l1_resource_bounds_mapping(),),
     Ok(())
 )]
 #[case::valid_invoke_tx(
     VALIDATOR_CONFIG_FOR_TESTING,
-    create_external_invoke_tx_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()),
     Ok(())
 )]
 fn test_transaction_validator(