From 20c296d2ab50f3e6576b2963ae9ea647ce263201 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Wo=C5=BAniak?= Date: Fri, 5 Jul 2024 11:53:51 +0200 Subject: [PATCH] L2 gas fix tx v3 (#528) --- crates/starknet-devnet-core/src/error.rs | 4 +- .../src/starknet/add_declare_transaction.rs | 18 +++++- .../add_deploy_account_transaction.rs | 12 +++- .../src/starknet/add_invoke_transaction.rs | 60 ++++++++++++++++++- .../src/rpc/transactions.rs | 26 ++++---- 5 files changed, 101 insertions(+), 19 deletions(-) diff --git a/crates/starknet-devnet-core/src/error.rs b/crates/starknet-devnet-core/src/error.rs index 976e7d280..2d7cb95f7 100644 --- 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 index 2e06d1be4..3d2f5020e 100644 --- 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 index 0159b105c..11546c90c 100644 --- 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 index 9c3af33e1..48b2b2d15 100644 --- 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::() .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::() + .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 index 7e9131af2..16b7f7518 100644 --- 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.