feat: add tx fee validation

starkware-libs · Mar 31, 2024 · b2dcffd · b2dcffd
1 parent 5f499ff
commit b2dcffd
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 13 deletions.
diff --git a/crates/gateway/src/errors.rs b/crates/gateway/src/errors.rs
@@ -1,4 +1,4 @@
-use starknet_api::transaction::TransactionVersion;
+use starknet_api::transaction::{Resource, ResourceBounds, TransactionVersion};
 
 use thiserror::Error;
 
@@ -29,6 +29,11 @@ pub enum TransactionValidatorError {
     BlockedTransactionVersion(TransactionVersion, String),
     #[error("This transaction type is not supported by the mempool")]
     TransactionTypeNotSupported,
+    #[error("Expected a positive amount of {resource:?}. Got {resource_bounds:?}.")]
+    ZeroFee {
+        resource: Resource,
+        resource_bounds: ResourceBounds,
+    },
     #[error(
         "Calldata length exceeded maximum: length {calldata_length}
         (allowed length: {max_calldata_length})."

diff --git a/crates/gateway/src/starknet_api_utils.rs b/crates/gateway/src/starknet_api_utils.rs
@@ -14,6 +14,7 @@ pub trait TransactionVersionExt {
 
 pub trait TransactionParametersExt {
     // Note, not all transaction types has a calldata field.
+    fn ref_to_resource_bounds(&self) -> TransactionValidatorResult<&ResourceBoundsMapping>;
     fn ref_to_calldata(&self) -> TransactionValidatorResult<&Calldata>;
     fn ref_to_signature(&self) -> TransactionValidatorResult<&TransactionSignature>;
     fn create_for_testing(
@@ -68,6 +69,7 @@ impl TransactionVersionExt for Transaction {
 
 impl TransactionParametersExt for Transaction {
     implement_transaction_params_getters!(
+        (ref_to_resource_bounds, ResourceBoundsMapping),
         (ref_to_calldata, Calldata),
         (ref_to_signature, TransactionSignature)
     );
@@ -90,7 +92,14 @@ impl TransactionParametersExt for DeclareTransaction {
         unimplemented!("Declare transactions do not have calldata.")
     }
 
-    implement_tx_params_ref_getters!((ref_to_signature, signature, TransactionSignature));
+    implement_tx_params_ref_getters!(
+        (
+            ref_to_resource_bounds,
+            resource_bounds,
+            ResourceBoundsMapping
+        ),
+        (ref_to_signature, signature, TransactionSignature)
+    );
 
     fn create_for_testing(
         resource_bounds: ResourceBoundsMapping,
@@ -116,6 +125,11 @@ impl TransactionParametersExt for DeclareTransaction {
 
 impl TransactionParametersExt for DeployAccountTransaction {
     implement_tx_params_ref_getters!(
+        (
+            ref_to_resource_bounds,
+            resource_bounds,
+            ResourceBoundsMapping
+        ),
         (ref_to_calldata, constructor_calldata, Calldata),
         (ref_to_signature, signature, TransactionSignature)
     );
@@ -142,6 +156,11 @@ impl TransactionParametersExt for DeployAccountTransaction {
 
 impl TransactionParametersExt for InvokeTransaction {
     implement_tx_params_ref_getters!(
+        (
+            ref_to_resource_bounds,
+            resource_bounds,
+            ResourceBoundsMapping
+        ),
         (ref_to_calldata, calldata, Calldata),
         (ref_to_signature, signature, TransactionSignature)
     );
@@ -181,3 +200,20 @@ pub fn zero_resource_bounds_mapping() -> ResourceBoundsMapping {
     ])
     .expect("Resource bounds mapping has unexpected structure.")
 }
+
+pub fn non_zero_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.")
+}
diff --git a/crates/gateway/src/transaction_validator.rs b/crates/gateway/src/transaction_validator.rs
@@ -90,12 +90,25 @@ impl TransactionValidator {
             return Err(TransactionValidatorError::TransactionTypeNotSupported);
         }
 
-        // TODO(Arni, 1/4/2024): Validate fee.
+        self.validate_fee(&tx)?;
         self.validate_tx_size(&tx)?;
 
         Ok(())
     }
 
+    fn validate_fee(&self, tx: &Transaction) -> TransactionValidatorResult<()> {
+        let resource = self.config.fee_resource;
+        let resource_bounds = tx.ref_to_resource_bounds()?.0[&resource];
+        if resource_bounds.max_amount == 0 || resource_bounds.max_price_per_unit == 0 {
+            return Err(TransactionValidatorError::ZeroFee {
+                resource,
+                resource_bounds,
+            });
+        }
+
+        Ok(())
+    }
+
     fn validate_tx_size(&self, tx: &Transaction) -> TransactionValidatorResult<()> {
         if let Transaction::DeployAccount(_) | Transaction::Invoke(_) = tx {
             let calldata = tx.ref_to_calldata()?;

diff --git a/crates/gateway/src/transaction_validator_test.rs b/crates/gateway/src/transaction_validator_test.rs
@@ -4,10 +4,12 @@ use starknet_api::calldata;
 use starknet_api::hash::StarkFelt;
 use starknet_api::transaction::{
     Calldata, DeclareTransaction, DeployAccountTransaction, InvokeTransaction, InvokeTransactionV1,
-    Resource, Transaction, TransactionSignature, TransactionVersion,
+    Resource, ResourceBounds, Transaction, TransactionSignature, TransactionVersion,
 };
 
-use crate::starknet_api_utils::{zero_resource_bounds_mapping, TransactionParametersExt};
+use crate::starknet_api_utils::{
+    non_zero_resource_bounds_mapping, zero_resource_bounds_mapping, TransactionParametersExt,
+};
 use crate::transaction_validator::{
     TransactionValidator, TransactionValidatorConfig, TransactionValidatorError,
     TransactionValidatorResult,
@@ -88,10 +90,19 @@ const VALIDATOR_CONFIG_FOR_TESTING: TransactionValidatorConfig = TransactionVali
     Transaction::L1Handler(starknet_api::transaction::L1HandlerTransaction {..Default::default()}),
     Err(TransactionValidatorError::TransactionTypeNotSupported)
 )]
+#[case::invalid_resource_bounds(
+    TransactionValidatorConfig {
+        min_allowed_tx_version: TransactionVersion::ZERO,
+        max_allowed_tx_version: TransactionVersion::THREE,
+        ..Default::default()
+    },
+    Transaction::create_for_testing(zero_resource_bounds_mapping(), None, None),
+    Err(TransactionValidatorError::ZeroFee{resource: Resource::L1Gas, resource_bounds: ResourceBounds::default()})
+)]
 #[case::deploy_account_calldata_too_long(
     VALIDATOR_CONFIG_FOR_TESTING,
     Transaction::DeployAccount(DeployAccountTransaction::create_for_testing(
-        zero_resource_bounds_mapping(),
+        non_zero_resource_bounds_mapping(),
         None,
         Some(calldata![StarkFelt::from_u128(1),StarkFelt::from_u128(2)])
     )),
@@ -100,7 +111,7 @@ const VALIDATOR_CONFIG_FOR_TESTING: TransactionValidatorConfig = TransactionVali
 #[case::invoke_calldata_too_long(
     VALIDATOR_CONFIG_FOR_TESTING,
     Transaction::Invoke(InvokeTransaction::create_for_testing(
-        zero_resource_bounds_mapping(),
+        non_zero_resource_bounds_mapping(),
         None,
         Some(calldata![StarkFelt::from_u128(1),StarkFelt::from_u128(2)])
     )),
@@ -109,7 +120,7 @@ const VALIDATOR_CONFIG_FOR_TESTING: TransactionValidatorConfig = TransactionVali
 #[case::declare_signature_too_long(
     VALIDATOR_CONFIG_FOR_TESTING,
     Transaction::Declare(DeclareTransaction::create_for_testing(
-        zero_resource_bounds_mapping(),
+        non_zero_resource_bounds_mapping(),
         Some(TransactionSignature(vec![StarkFelt::from_u128(1),StarkFelt::from_u128(2)])),
         None
     )),
@@ -119,7 +130,7 @@ const VALIDATOR_CONFIG_FOR_TESTING: TransactionValidatorConfig = TransactionVali
 #[case::deploy_account_signature_too_long(
     VALIDATOR_CONFIG_FOR_TESTING,
     Transaction::DeployAccount(DeployAccountTransaction::create_for_testing(
-        zero_resource_bounds_mapping(),
+        non_zero_resource_bounds_mapping(),
         Some(TransactionSignature(vec![StarkFelt::from_u128(1),StarkFelt::from_u128(2)])),
         None
     )),
@@ -128,7 +139,7 @@ const VALIDATOR_CONFIG_FOR_TESTING: TransactionValidatorConfig = TransactionVali
 #[case::invoke_signature_too_long(
     VALIDATOR_CONFIG_FOR_TESTING,
     Transaction::Invoke(InvokeTransaction::create_for_testing(
-        zero_resource_bounds_mapping(),
+        non_zero_resource_bounds_mapping(),
         Some(TransactionSignature(vec![StarkFelt::from_u128(1),StarkFelt::from_u128(2)])),
         None
     )),
@@ -137,21 +148,21 @@ const VALIDATOR_CONFIG_FOR_TESTING: TransactionValidatorConfig = TransactionVali
 #[case::valid_declare_tx(
     VALIDATOR_CONFIG_FOR_TESTING,
     Transaction::Declare(DeclareTransaction::create_for_testing(
-        zero_resource_bounds_mapping(), None, None
+        non_zero_resource_bounds_mapping(), None, None
     )),
     Ok(())
 )]
 #[case::valid_deploy_account_tx(
     VALIDATOR_CONFIG_FOR_TESTING,
     Transaction::DeployAccount(DeployAccountTransaction::create_for_testing(
-        zero_resource_bounds_mapping(), None, None
+        non_zero_resource_bounds_mapping(), None, None
     )),
     Ok(())
 )]
 #[case::valid_invoke_tx(
     VALIDATOR_CONFIG_FOR_TESTING,
     Transaction::Invoke(InvokeTransaction::create_for_testing(
-        zero_resource_bounds_mapping(), None, None
+        non_zero_resource_bounds_mapping(), None, None
     )),
     Ok(())
 )]