test(starknet_api): refactor executable_deploy_account_tx test util (#…

…3610)

crates/blockifier/src/blockifier/transaction_executor_test.rs

@@ -4,7 +4,7 @@ use rstest::rstest;
 use starknet_api::test_utils::declare::executable_declare_tx;
 use starknet_api::test_utils::deploy_account::executable_deploy_account_tx;
 use starknet_api::test_utils::invoke::executable_invoke_tx;
-use starknet_api::test_utils::{NonceManager, DEFAULT_STRK_L1_GAS_PRICE};
+use starknet_api::test_utils::DEFAULT_STRK_L1_GAS_PRICE;
 use starknet_api::transaction::fields::Fee;
 use starknet_api::transaction::TransactionVersion;
 use starknet_api::{declare_tx_args, deploy_account_tx_args, felt, invoke_tx_args, nonce};
@@ -41,6 +41,7 @@ use crate::transaction::test_utils::{
     TestInitData,
 };
 use crate::transaction::transaction_execution::Transaction;
+
 fn tx_executor_test_body<S: StateReader>(
     state: CachedState<S>,
     block_context: BlockContext,
@@ -146,14 +147,11 @@ fn test_deploy_account(
     let account_contract = FeatureContract::AccountWithoutValidations(cairo_version);
     let state = test_state(&block_context.chain_info, BALANCE, &[(account_contract, 0)]);
 
-    let deploy_account_tx = executable_deploy_account_tx(
-        deploy_account_tx_args! {
-            class_hash: account_contract.get_class_hash(),
-            resource_bounds: l1_resource_bounds(0_u8.into(), DEFAULT_STRK_L1_GAS_PRICE.into()),
-            version,
-        },
-        &mut NonceManager::default(),
-    );
+    let deploy_account_tx = executable_deploy_account_tx(deploy_account_tx_args! {
+        class_hash: account_contract.get_class_hash(),
+        resource_bounds: l1_resource_bounds(0_u8.into(), DEFAULT_STRK_L1_GAS_PRICE.into()),
+        version,
+    });
     let tx = AccountTransaction::new_for_sequencing(deploy_account_tx).into();
     let expected_bouncer_weights = BouncerWeights {
         state_diff_size: 3,

crates/blockifier/src/concurrency/versioned_state_test.rs

@@ -7,7 +7,7 @@ use rstest::{fixture, rstest};
 use starknet_api::abi::abi_utils::{get_fee_token_var_address, get_storage_var_address};
 use starknet_api::core::{calculate_contract_address, ClassHash, ContractAddress};
 use starknet_api::test_utils::deploy_account::executable_deploy_account_tx;
-use starknet_api::test_utils::{NonceManager, DEFAULT_STRK_L1_GAS_PRICE};
+use starknet_api::test_utils::DEFAULT_STRK_L1_GAS_PRICE;
 use starknet_api::transaction::fields::{ContractAddressSalt, ValidResourceBounds};
 use starknet_api::{
     calldata,
@@ -227,16 +227,13 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) {
     let mut state_2 = TransactionalState::create_transactional(&mut versioned_state_proxy_2);
 
     // Prepare transactions
-    let tx = executable_deploy_account_tx(
-        deploy_account_tx_args! {
-            class_hash: account_without_validation.get_class_hash(),
-            resource_bounds: l1_resource_bounds(
-                u8::from(!zero_bounds).into(),
-                DEFAULT_STRK_L1_GAS_PRICE.into()
-            ),
-        },
-        &mut NonceManager::default(),
-    );
+    let tx = executable_deploy_account_tx(deploy_account_tx_args! {
+        class_hash: account_without_validation.get_class_hash(),
+        resource_bounds: l1_resource_bounds(
+            u8::from(!zero_bounds).into(),
+            DEFAULT_STRK_L1_GAS_PRICE.into()
+        ),
+    });
     let deploy_account_tx_1 = AccountTransaction::new_for_sequencing(tx);
     let enforce_fee = deploy_account_tx_1.enforce_fee();
 
@@ -249,8 +246,7 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) {
         resource_bounds: default_all_resource_bounds,
         constructor_calldata: constructor_calldata.clone(),
     };
-    let nonce_manager = &mut NonceManager::default();
-    let tx = executable_deploy_account_tx(deploy_tx_args, nonce_manager);
+    let tx = executable_deploy_account_tx(deploy_tx_args);
     let delpoy_account_tx_2 = AccountTransaction::new_for_sequencing(tx);
 
     let account_address = delpoy_account_tx_2.sender_address();

crates/blockifier/src/transaction/account_transactions_test.rs

@@ -203,27 +203,24 @@ fn test_fee_enforcement(
 ) {
     let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo0);
     let state = &mut test_state(&block_context.chain_info, BALANCE, &[(account, 1)]);
-    let tx = executable_deploy_account_tx(
-        deploy_account_tx_args! {
-            class_hash: account.get_class_hash(),
-            max_fee: Fee(if zero_bounds { 0 } else { MAX_FEE.0 }),
-            resource_bounds: match gas_bounds_mode {
-                GasVectorComputationMode::NoL2Gas => l1_resource_bounds(
-                    (if zero_bounds { 0 } else { DEFAULT_L1_GAS_AMOUNT.0 }).into(),
-                    DEFAULT_STRK_L1_GAS_PRICE.into()
-                ),
-                GasVectorComputationMode::All => create_gas_amount_bounds_with_default_price(
-                    GasVector{
-                        l1_gas: (if zero_bounds { 0 } else { DEFAULT_L1_GAS_AMOUNT.0 }).into(),
-                        l2_gas: (if zero_bounds { 0 } else { DEFAULT_L2_GAS_MAX_AMOUNT.0 }).into(),
-                        l1_data_gas: (if zero_bounds { 0 } else { DEFAULT_L1_DATA_GAS_MAX_AMOUNT.0 }).into(),
-                    },
-                ),
-            },
-            version,
+    let tx = executable_deploy_account_tx(deploy_account_tx_args! {
+        class_hash: account.get_class_hash(),
+        max_fee: Fee(if zero_bounds { 0 } else { MAX_FEE.0 }),
+        resource_bounds: match gas_bounds_mode {
+            GasVectorComputationMode::NoL2Gas => l1_resource_bounds(
+                (if zero_bounds { 0 } else { DEFAULT_L1_GAS_AMOUNT.0 }).into(),
+                DEFAULT_STRK_L1_GAS_PRICE.into()
+            ),
+            GasVectorComputationMode::All => create_gas_amount_bounds_with_default_price(
+                GasVector{
+                    l1_gas: (if zero_bounds { 0 } else { DEFAULT_L1_GAS_AMOUNT.0 }).into(),
+                    l2_gas: (if zero_bounds { 0 } else { DEFAULT_L2_GAS_MAX_AMOUNT.0 }).into(),
+                    l1_data_gas: (if zero_bounds { 0 } else { DEFAULT_L1_DATA_GAS_MAX_AMOUNT.0 }).into(),
+                },
+            ),
         },
-        &mut NonceManager::default(),
-    );
+        version,
+    });
     let deploy_account_tx = AccountTransaction::new_for_sequencing(tx);
 
     let enforce_fee = deploy_account_tx.enforce_fee();

crates/blockifier/src/transaction/test_utils.rs

@@ -5,7 +5,10 @@ use starknet_api::contract_class::{ClassInfo, ContractClass, SierraVersion};
 use starknet_api::core::{ClassHash, ContractAddress, Nonce};
 use starknet_api::execution_resources::{GasAmount, GasVector};
 use starknet_api::test_utils::declare::executable_declare_tx;
-use starknet_api::test_utils::deploy_account::{executable_deploy_account_tx, DeployAccountTxArgs};
+use starknet_api::test_utils::deploy_account::{
+    create_executable_deploy_account_tx_and_update_nonce,
+    DeployAccountTxArgs,
+};
 use starknet_api::test_utils::invoke::{executable_invoke_tx, InvokeTxArgs};
 use starknet_api::test_utils::{
     NonceManager,
@@ -127,7 +130,7 @@ pub fn deploy_and_fund_account(
 ) -> (AccountTransaction, ContractAddress) {
     // Deploy an account contract.
     let deploy_account_tx = AccountTransaction::new_with_default_flags(
-        executable_deploy_account_tx(deploy_tx_args, nonce_manager),
+        create_executable_deploy_account_tx_and_update_nonce(deploy_tx_args, nonce_manager),
     );
     let account_address = deploy_account_tx.sender_address();
 
@@ -281,7 +284,7 @@ pub fn create_account_tx_for_validate_test(
                 true => constants::FELT_TRUE,
                 false => constants::FELT_FALSE,
             })];
-            let tx = executable_deploy_account_tx(
+            let tx = create_executable_deploy_account_tx_and_update_nonce(
                 deploy_account_tx_args! {
                     max_fee,
                     resource_bounds,

crates/blockifier/src/transaction/transactions_test.rs

@@ -21,7 +21,11 @@ use starknet_api::executable_transaction::AccountTransaction as ApiExecutableTra
 use starknet_api::execution_resources::{GasAmount, GasVector};
 use starknet_api::state::StorageKey;
 use starknet_api::test_utils::declare::executable_declare_tx;
-use starknet_api::test_utils::deploy_account::{executable_deploy_account_tx, DeployAccountTxArgs};
+use starknet_api::test_utils::deploy_account::{
+    create_executable_deploy_account_tx_and_update_nonce,
+    executable_deploy_account_tx,
+    DeployAccountTxArgs,
+};
 use starknet_api::test_utils::invoke::{executable_invoke_tx, InvokeTxArgs};
 use starknet_api::test_utils::{
     NonceManager,
@@ -1036,7 +1040,6 @@ fn test_max_fee_exceeds_balance(
             resource_bounds,
             class_hash: FeatureContract::TestContract(cairo_version).get_class_hash()
         },
-        &mut NonceManager::default(),
     ));
     assert_resource_bounds_exceed_balance_failure(&mut state, &block_context, invalid_tx);
 
@@ -1812,13 +1815,15 @@ fn test_deploy_account_tx(
     let account = FeatureContract::AccountWithoutValidations(cairo_version);
     let account_class_hash = account.get_class_hash();
     let state = &mut test_state(chain_info, BALANCE, &[(account, 1)]);
-    let deploy_account = AccountTransaction::new_with_default_flags(executable_deploy_account_tx(
-        deploy_account_tx_args! {
-            resource_bounds: default_all_resource_bounds,
-            class_hash: account_class_hash
-        },
-        &mut nonce_manager,
-    ));
+    let deploy_account = AccountTransaction::new_with_default_flags(
+        create_executable_deploy_account_tx_and_update_nonce(
+            deploy_account_tx_args! {
+                resource_bounds: default_all_resource_bounds,
+                class_hash: account_class_hash
+            },
+            &mut nonce_manager,
+        ),
+    );
 
     // Extract deploy account transaction fields for testing, as it is consumed when creating an
     // account transaction.
@@ -1973,13 +1978,15 @@ fn test_deploy_account_tx(
 
     // Negative flow.
     // Deploy to an existing address.
-    let deploy_account = AccountTransaction::new_with_default_flags(executable_deploy_account_tx(
-        deploy_account_tx_args! {
-            resource_bounds: default_all_resource_bounds,
-            class_hash: account_class_hash
-        },
-        &mut nonce_manager,
-    ));
+    let deploy_account = AccountTransaction::new_with_default_flags(
+        create_executable_deploy_account_tx_and_update_nonce(
+            deploy_account_tx_args! {
+                resource_bounds: default_all_resource_bounds,
+                class_hash: account_class_hash
+            },
+            &mut nonce_manager,
+        ),
+    );
     let error = deploy_account.execute(state, block_context).unwrap_err();
     assert_matches!(
         error,
@@ -2002,13 +2009,11 @@ fn test_fail_deploy_account_undeclared_class_hash(
     let block_context = &block_context;
     let chain_info = &block_context.chain_info;
     let state = &mut test_state(chain_info, BALANCE, &[]);
-    let mut nonce_manager = NonceManager::default();
     let undeclared_hash = class_hash!("0xdeadbeef");
     let deploy_account = AccountTransaction::new_with_default_flags(executable_deploy_account_tx(
         deploy_account_tx_args! {
             resource_bounds: default_all_resource_bounds,  class_hash: undeclared_hash
         },
-        &mut nonce_manager,
     ));
     let tx_context = block_context.to_tx_context(&deploy_account);
     let fee_type = tx_context.tx_info.fee_type();
@@ -2847,7 +2852,6 @@ fn test_deploy_max_sierra_gas_validate_execute(
     #[case] tx_args: DeployAccountTxArgs,
 ) {
     let chain_info = &block_context.chain_info;
-    let mut nonce_manager = NonceManager::default();
     let account = FeatureContract::AccountWithoutValidations(cairo_version);
     let account_class_hash = account.get_class_hash();
     let state = &mut test_state(chain_info, BALANCE, &[(account, 1)]);
@@ -2856,7 +2860,6 @@ fn test_deploy_max_sierra_gas_validate_execute(
             class_hash: account_class_hash,
             .. tx_args
         },
-        &mut nonce_manager,
     ));
 
     // Extract deploy account transaction fields for testing, as it is consumed when creating an

crates/starknet_api/src/test_utils/deploy_account.rs

@@ -116,10 +116,21 @@ pub fn deploy_account_tx(
     }
 }
 
+// TODO(Arni): Consider using [ExecutableDeployAccountTransaction::create] in the body of this
+// function. We don't use it now to avoid tx_hash calculation.
+pub fn executable_deploy_account_tx(deploy_tx_args: DeployAccountTxArgs) -> AccountTransaction {
+    let tx_hash = deploy_tx_args.tx_hash;
+    let tx = deploy_account_tx(deploy_tx_args, Nonce(Felt::ZERO));
+    let contract_address = tx.calculate_contract_address().unwrap();
+    let deploy_account_tx = ExecutableDeployAccountTransaction { tx, tx_hash, contract_address };
+
+    AccountTransaction::DeployAccount(deploy_account_tx)
+}
+
 // TODO(Arni): Consider using [ExecutableDeployAccountTransaction::create] in the body of this
 // function. We don't use it now to avoid tx_hash calculation.
 // TODO(Arni): Streamline this function by using nonce = 0 always.
-pub fn executable_deploy_account_tx(
+pub fn create_executable_deploy_account_tx_and_update_nonce(
     deploy_tx_args: DeployAccountTxArgs,
     nonce_manager: &mut NonceManager,
 ) -> AccountTransaction {

crates/starknet_gateway/src/stateful_transaction_validator_test.rs

@@ -22,7 +22,6 @@ use starknet_api::executable_transaction::AccountTransaction;
 use starknet_api::execution_resources::GasAmount;
 use starknet_api::test_utils::deploy_account::executable_deploy_account_tx;
 use starknet_api::test_utils::invoke::executable_invoke_tx;
-use starknet_api::test_utils::NonceManager;
 use starknet_api::transaction::fields::Resource;
 use starknet_api::{deploy_account_tx_args, invoke_tx_args, nonce};
 use starknet_gateway_types::errors::GatewaySpecError;
@@ -141,8 +140,10 @@ fn test_instantiate_validator(stateful_validator: StatefulTransactionValidator)
     true,
     false
 )]
+// TODO(Arni): Fix this test case. Ideally, we would have a non-invoke transaction with tx_nonce 1
+// and account_nonce 0. For deploy account the tx_nonce is always 0. Replace with a declare tx.
 #[case::should_not_skip_validation_non_invoke(
-    executable_deploy_account_tx(deploy_account_tx_args!(), &mut NonceManager::default()),
+    executable_deploy_account_tx(deploy_account_tx_args!()),
     nonce!(0),
     true,
     false