L2 gas fix tx v3 (#528)

0xSpaceShard · Jul 5, 2024 · 20c296d · 20c296d
1 parent c9dd6df
commit 20c296d
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 19 deletions.
diff --git a/crates/starknet-devnet-core/src/error.rs b/crates/starknet-devnet-core/src/error.rs
@@ -55,7 +55,9 @@ pub enum Error {
     SerializationError { origin: String },
     #[error("Serialization not supported: {obj_name}")]
     SerializationNotSupported { obj_name: String },
-    #[error("{tx_type}: max_fee cannot be zero")]
+    #[error(
+        "{tx_type}: max_fee cannot be zero (exception is v3 transaction where l2 gas must be zero)"
+    )]
     MaxFeeZeroError { tx_type: String },
     #[error(transparent)]
     TransactionValidationError(#[from] TransactionValidationError),

diff --git a/crates/starknet-devnet-core/src/starknet/add_declare_transaction.rs b/crates/starknet-devnet-core/src/starknet/add_declare_transaction.rs
@@ -183,7 +183,11 @@ mod tests {
         assert!(result.is_err());
         match result.err().unwrap() {
             err @ crate::error::Error::MaxFeeZeroError { .. } => {
-                assert_eq!(err.to_string(), "Declare transaction V3: max_fee cannot be zero")
+                assert_eq!(
+                    err.to_string(),
+                    "Declare transaction V3: max_fee cannot be zero (exception is v3 transaction \
+                     where l2 gas must be zero)"
+                )
             }
             _ => panic!("Wrong error type"),
         }
@@ -208,7 +212,11 @@ mod tests {
         assert!(result.is_err());
         match result.err().unwrap() {
             err @ crate::error::Error::MaxFeeZeroError { .. } => {
-                assert_eq!(err.to_string(), "Declare transaction V2: max_fee cannot be zero")
+                assert_eq!(
+                    err.to_string(),
+                    "Declare transaction V2: max_fee cannot be zero (exception is v3 transaction \
+                     where l2 gas must be zero)"
+                )
             }
             _ => panic!("Wrong error type"),
         }
@@ -344,7 +352,11 @@ mod tests {
         assert!(result.is_err());
         match result.err().unwrap() {
             err @ crate::error::Error::MaxFeeZeroError { .. } => {
-                assert_eq!(err.to_string(), "Declare transaction V1: max_fee cannot be zero")
+                assert_eq!(
+                    err.to_string(),
+                    "Declare transaction V1: max_fee cannot be zero (exception is v3 transaction \
+                     where l2 gas must be zero)"
+                )
             }
             _ => panic!("Wrong error type"),
         }

diff --git a/crates/starknet-devnet-core/src/starknet/add_deploy_account_transaction.rs b/crates/starknet-devnet-core/src/starknet/add_deploy_account_transaction.rs
@@ -166,7 +166,11 @@ mod tests {
         assert!(result.is_err());
         match result.err().unwrap() {
             err @ crate::error::Error::MaxFeeZeroError { .. } => {
-                assert_eq!(err.to_string(), "Deploy account transaction V1: max_fee cannot be zero")
+                assert_eq!(
+                    err.to_string(),
+                    "Deploy account transaction V1: max_fee cannot be zero (exception is v3 \
+                     transaction where l2 gas must be zero)"
+                )
             }
             _ => panic!("Wrong error type"),
         }
@@ -185,7 +189,11 @@ mod tests {
             .unwrap_err();
         match txn_err {
             err @ crate::error::Error::MaxFeeZeroError { .. } => {
-                assert_eq!(err.to_string(), "Deploy account transaction V3: max_fee cannot be zero")
+                assert_eq!(
+                    err.to_string(),
+                    "Deploy account transaction V3: max_fee cannot be zero (exception is v3 \
+                     transaction where l2 gas must be zero)"
+                )
             }
             _ => panic!("Wrong error type"),
         }

diff --git a/crates/starknet-devnet-core/src/starknet/add_invoke_transaction.rs b/crates/starknet-devnet-core/src/starknet/add_invoke_transaction.rs
@@ -133,6 +133,7 @@ mod tests {
         param: Felt,
         nonce: u128,
         l1_gas_amount: u64,
+        l2_gas_amount: u64,
     ) -> BroadcastedInvokeTransaction {
         let calldata = vec![
             Felt::from(contract_address), // contract address
@@ -146,7 +147,7 @@ mod tests {
                 version: Felt::from(3),
                 signature: vec![],
                 nonce: Felt::from(nonce),
-                resource_bounds: ResourceBoundsWrapper::new(l1_gas_amount, 1, 0, 0),
+                resource_bounds: ResourceBoundsWrapper::new(l1_gas_amount, 1, l2_gas_amount, 1),
                 tip: Tip(0),
                 paymaster_data: vec![],
                 nonce_data_availability_mode:
@@ -169,6 +170,7 @@ mod tests {
             Felt::from(10),
             0,
             1,
+            0,
         );
         match invoke_transaction {
             BroadcastedInvokeTransaction::V3(ref mut v3) => {
@@ -200,6 +202,7 @@ mod tests {
             Felt::from(10),
             0,
             0,
+            0,
         );
 
         let invoke_v3_txn_error = starknet
@@ -210,7 +213,9 @@ mod tests {
             err @ crate::error::Error::MaxFeeZeroError { .. } => {
                 assert_eq!(
                     err.to_string(),
-                    "Invoke transaction V3: max_fee cannot be zero".to_string()
+                    "Invoke transaction V3: max_fee cannot be zero (exception is v3 transaction \
+                     where l2 gas must be zero)"
+                        .to_string()
                 );
             }
             _ => panic!("Wrong error type"),
@@ -236,6 +241,7 @@ mod tests {
                 .to_string()
                 .parse::<u64>()
                 .unwrap(),
+            0,
         );
 
         let transaction_hash = starknet.add_invoke_transaction(invoke_transaction).unwrap();
@@ -250,6 +256,50 @@ mod tests {
         );
     }
 
+    #[test]
+    fn invoke_transaction_v3_positive_l2_gas_should_fail() {
+        let (mut starknet, account, contract_address, increase_balance_selector, _) = setup();
+        let account_address = account.get_address();
+
+        let l1_gas = account
+            .get_balance(&mut starknet.pending_state, crate::account::FeeToken::STRK)
+            .unwrap()
+            .to_string()
+            .parse::<u64>()
+            .unwrap();
+
+        // l2 gas should always be set to zero and l1 gas should be greater than 0 for v3
+        // transactions, this is why these 2 cases should fail
+        let fail_test_cases = [(l1_gas, 1), (0, 1)];
+        for test_case in fail_test_cases {
+            let invoke_transaction = test_invoke_transaction_v3(
+                account_address,
+                contract_address,
+                increase_balance_selector,
+                Felt::from(10),
+                0,
+                test_case.0,
+                test_case.1,
+            );
+
+            let transaction = starknet.add_invoke_transaction(invoke_transaction);
+
+            assert!(transaction.is_err());
+            match transaction.err().unwrap() {
+                err @ crate::error::Error::MaxFeeZeroError { .. } => {
+                    assert_eq!(
+                        err.to_string(),
+                        "Invoke transaction V3: max_fee cannot be zero (exception is v3 \
+                         transaction where l2 gas must be zero)"
+                    )
+                }
+                _ => {
+                    panic!("Wrong error type")
+                }
+            }
+        }
+    }
+
     #[test]
     fn invoke_transaction_successful_execution() {
         let (mut starknet, account, contract_address, increase_balance_selector, _) = setup();
@@ -344,7 +394,11 @@ mod tests {
         assert!(result.is_err());
         match result.err().unwrap() {
             err @ crate::error::Error::MaxFeeZeroError { .. } => {
-                assert_eq!(err.to_string(), "Invoke transaction V1: max_fee cannot be zero")
+                assert_eq!(
+                    err.to_string(),
+                    "Invoke transaction V1: max_fee cannot be zero (exception is v3 transaction \
+                     where l2 gas must be zero)"
+                )
             }
             _ => panic!("Wrong error type"),
         }

diff --git a/crates/starknet-devnet-types/src/rpc/transactions.rs b/crates/starknet-devnet-types/src/rpc/transactions.rs
@@ -342,15 +342,19 @@ impl From<&ResourceBoundsWrapper> for starknet_api::transaction::ResourceBoundsM
 }
 
 impl BroadcastedTransactionCommonV3 {
-    /// Checks if total accumulated fee of resource_bounds is equal to 0
-    pub fn is_max_fee_zero_value(&self) -> bool {
-        (self.resource_bounds.inner.l1_gas.max_amount as u128)
+    /// Checks if total accumulated fee of resource_bounds for l1 is equal to 0 or for l2 is not
+    /// zero
+    pub fn is_l1_gas_zero_or_l2_gas_not_zero(&self) -> bool {
+        let l2_is_not_zero = (self.resource_bounds.inner.l2_gas.max_amount as u128)
+            * self.resource_bounds.inner.l2_gas.max_price_per_unit
+            > 0;
+        let l1_is_zero = (self.resource_bounds.inner.l1_gas.max_amount as u128)
             * self.resource_bounds.inner.l1_gas.max_price_per_unit
-            == 0
-            && (self.resource_bounds.inner.l2_gas.max_amount as u128)
-                * self.resource_bounds.inner.l2_gas.max_price_per_unit
-                == 0
+            == 0;
+
+        l1_is_zero || l2_is_not_zero
     }
+
     /// Returns an array of FieldElements that reflects the `common_tx_fields` according to SNIP-8(https://github.com/starknet-io/SNIPs/blob/main/SNIPS/snip-8.md/#protocol-changes).
     ///
     /// # Arguments
@@ -515,7 +519,7 @@ impl BroadcastedDeclareTransaction {
         match self {
             BroadcastedDeclareTransaction::V1(v1) => v1.common.is_max_fee_zero_value(),
             BroadcastedDeclareTransaction::V2(v2) => v2.common.is_max_fee_zero_value(),
-            BroadcastedDeclareTransaction::V3(v3) => v3.common.is_max_fee_zero_value(),
+            BroadcastedDeclareTransaction::V3(v3) => v3.common.is_l1_gas_zero_or_l2_gas_not_zero(),
         }
     }
     /// Creates a blockifier declare transaction from the current transaction.
@@ -658,7 +662,9 @@ impl BroadcastedDeployAccountTransaction {
     pub fn is_max_fee_zero_value(&self) -> bool {
         match self {
             BroadcastedDeployAccountTransaction::V1(v1) => v1.common.is_max_fee_zero_value(),
-            BroadcastedDeployAccountTransaction::V3(v3) => v3.common.is_max_fee_zero_value(),
+            BroadcastedDeployAccountTransaction::V3(v3) => {
+                v3.common.is_l1_gas_zero_or_l2_gas_not_zero()
+            }
         }
     }
     /// Creates a blockifier deploy account transaction from the current transaction.
@@ -793,7 +799,7 @@ impl BroadcastedInvokeTransaction {
     pub fn is_max_fee_zero_value(&self) -> bool {
         match self {
             BroadcastedInvokeTransaction::V1(v1) => v1.common.is_max_fee_zero_value(),
-            BroadcastedInvokeTransaction::V3(v3) => v3.common.is_max_fee_zero_value(),
+            BroadcastedInvokeTransaction::V3(v3) => v3.common.is_l1_gas_zero_or_l2_gas_not_zero(),
         }
     }
     /// Creates a blockifier invoke transaction from the current transaction.