refactor(blockifier): change return type of invoke_tx deploy_account_tx to account_transaction

crates/blockifier/src/concurrency/versioned_state_test.rs

@@ -44,7 +44,6 @@ use crate::test_utils::deploy_account::deploy_account_tx;
 use crate::test_utils::dict_state_reader::DictStateReader;
 use crate::test_utils::initial_test_state::test_state;
 use crate::test_utils::{CairoVersion, BALANCE, DEFAULT_STRK_L1_GAS_PRICE};
-use crate::transaction::account_transaction::AccountTransaction;
 use crate::transaction::objects::HasRelatedFeeType;
 use crate::transaction::test_utils::{default_all_resource_bounds, l1_resource_bounds};
 use crate::transaction::transactions::ExecutableTransaction;
@@ -239,8 +238,7 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) {
         },
         &mut NonceManager::default(),
     );
-    let account_tx_1 = AccountTransaction::DeployAccount(deploy_account_tx_1);
-    let enforce_fee = account_tx_1.enforce_fee();
+    let enforce_fee = deploy_account_tx_1.enforce_fee();
 
     let class_hash = grindy_account.get_class_hash();
     let ctor_storage_arg = felt!(1_u8);
@@ -252,10 +250,9 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) {
         constructor_calldata: constructor_calldata.clone(),
     };
     let nonce_manager = &mut NonceManager::default();
-    let deploy_account_tx_2 = deploy_account_tx(deploy_tx_args, nonce_manager);
-    let account_address = deploy_account_tx_2.contract_address();
-    let account_tx_2 = AccountTransaction::DeployAccount(deploy_account_tx_2);
-    let tx_context = block_context.to_tx_context(&account_tx_2);
+    let delpoy_account_tx_2 = deploy_account_tx(deploy_tx_args, nonce_manager);
+    let account_address = delpoy_account_tx_2.sender_address();
+    let tx_context = block_context.to_tx_context(&delpoy_account_tx_2);
     let fee_type = tx_context.tx_info.fee_type();
 
     let deployed_account_balance_key = get_fee_token_var_address(account_address);
@@ -270,11 +267,12 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) {
     // Execute transactions
     thread::scope(|s| {
         s.spawn(move || {
-            let result = account_tx_1.execute(&mut state_1, &block_context_1, enforce_fee, true);
+            let result =
+                deploy_account_tx_1.execute(&mut state_1, &block_context_1, enforce_fee, true);
             assert_eq!(result.is_err(), enforce_fee); // Transaction fails iff we enforced the fee charge (as the acount is not funded).
         });
         s.spawn(move || {
-            account_tx_2.execute(&mut state_2, &block_context_2, true, true).unwrap();
+            delpoy_account_tx_2.execute(&mut state_2, &block_context_2, true, true).unwrap();
             // Check that the constructor wrote ctor_arg to the storage.
             let storage_key = get_storage_var_address("ctor_arg", &[]);
             let deployed_contract_address = calculate_contract_address(

crates/blockifier/src/test_utils/deploy_account.rs

@@ -1,15 +1,20 @@
 use starknet_api::test_utils::deploy_account::DeployAccountTxArgs;
 use starknet_api::test_utils::NonceManager;
 
+use crate::transaction::account_transaction::AccountTransaction;
 use crate::transaction::transactions::DeployAccountTransaction;
 
 pub fn deploy_account_tx(
     deploy_tx_args: DeployAccountTxArgs,
     nonce_manager: &mut NonceManager,
-) -> DeployAccountTransaction {
+) -> AccountTransaction {
     let deploy_account_tx = starknet_api::test_utils::deploy_account::executable_deploy_account_tx(
         deploy_tx_args,
         nonce_manager,
     );
-    DeployAccountTransaction { tx: deploy_account_tx, only_query: false }
+
+    // TODO(AvivG): use the "new" method.
+    let executable_deploy_account_tx =
+        DeployAccountTransaction { tx: deploy_account_tx, only_query: false };
+    AccountTransaction::DeployAccount(executable_deploy_account_tx)
 }

crates/blockifier/src/test_utils/invoke.rs

@@ -2,10 +2,11 @@ use starknet_api::test_utils::invoke::InvokeTxArgs;
 use starknet_api::transaction::{InvokeTransactionV0, TransactionVersion};
 
 use crate::abi::abi_utils::selector_from_name;
+use crate::transaction::account_transaction::AccountTransaction;
 use crate::transaction::constants::EXECUTE_ENTRY_POINT_NAME;
 use crate::transaction::transactions::InvokeTransaction;
 
-pub fn invoke_tx(invoke_args: InvokeTxArgs) -> InvokeTransaction {
+pub fn invoke_tx(invoke_args: InvokeTxArgs) -> AccountTransaction {
     let tx_hash = invoke_args.tx_hash;
     let only_query = invoke_args.only_query;
     // TODO: Make TransactionVersion an enum and use match here.
@@ -22,8 +23,9 @@ pub fn invoke_tx(invoke_args: InvokeTxArgs) -> InvokeTransaction {
         starknet_api::test_utils::invoke::invoke_tx(invoke_args)
     };
 
-    match only_query {
+    let invoke_tx = match only_query {
         true => InvokeTransaction::new_for_query(invoke_tx, tx_hash),
         false => InvokeTransaction::new(invoke_tx, tx_hash),
-    }
+    };
+    AccountTransaction::Invoke(invoke_tx)
 }

crates/blockifier/src/test_utils/transfers_generator.rs

@@ -179,14 +179,13 @@ impl TransfersGenerator {
             felt!(0_u8)                 // Calldata: msb amount.
         ];
 
-        let tx = invoke_tx(invoke_tx_args! {
+        invoke_tx(invoke_tx_args! {
             max_fee: self.config.max_fee,
             sender_address,
             calldata: execute_calldata,
             version: self.config.tx_version,
             nonce,
-        });
-        AccountTransaction::Invoke(tx)
+        })
     }
 }
 

crates/blockifier/src/transaction/account_transaction.rs

@@ -229,7 +229,7 @@ impl AccountTransaction {
 
     pub fn calldata_length(&self) -> usize {
         let calldata = match self {
-            Self::Declare(_tx) => calldata![],
+            Self::Declare(_tx) => return 0,
             Self::DeployAccount(tx) => tx.constructor_calldata(),
             Self::Invoke(tx) => tx.calldata(),
         };

crates/blockifier/src/transaction/account_transactions_test.rs

@@ -205,10 +205,9 @@ fn test_fee_enforcement(
         &mut NonceManager::default(),
     );
 
-    let account_tx = AccountTransaction::DeployAccount(deploy_account_tx);
-    let enforce_fee = account_tx.create_tx_info().enforce_fee();
+    let enforce_fee = deploy_account_tx.create_tx_info().enforce_fee();
     assert_ne!(zero_bounds, enforce_fee);
-    let result = account_tx.execute(state, &block_context, enforce_fee, true);
+    let result = deploy_account_tx.execute(state, &block_context, enforce_fee, true);
     // Execution should fail if the fee is enforced because the account doesn't have sufficient
     // balance.
     assert_eq!(result.is_err(), enforce_fee);

crates/blockifier/src/transaction/test_utils.rs

@@ -139,8 +139,7 @@ pub fn deploy_and_fund_account(
 ) -> (AccountTransaction, ContractAddress) {
     // Deploy an account contract.
     let deploy_account_tx = deploy_account_tx(deploy_tx_args, nonce_manager);
-    let account_address = deploy_account_tx.contract_address();
-    let account_tx = AccountTransaction::DeployAccount(deploy_account_tx);
+    let account_address = deploy_account_tx.sender_address();
 
     // Update the balance of the about-to-be deployed account contract in the erc20 contract, so it
     // can pay for the transaction execution.
@@ -153,7 +152,7 @@ pub fn deploy_and_fund_account(
             .unwrap();
     }
 
-    (account_tx, account_address)
+    (deploy_account_tx, account_address)
 }
 
 /// Initializes a state and returns a `TestInitData` instance.
@@ -282,7 +281,7 @@ pub fn create_account_tx_for_validate_test(
                 true => constants::FELT_TRUE,
                 false => constants::FELT_FALSE,
             })];
-            let deploy_account_tx = deploy_account_tx(
+            deploy_account_tx(
                 deploy_account_tx_args! {
                     max_fee,
                     resource_bounds,
@@ -293,28 +292,26 @@ pub fn create_account_tx_for_validate_test(
                     constructor_calldata,
                 },
                 nonce_manager,
-            );
-            AccountTransaction::DeployAccount(deploy_account_tx)
+            )
         }
         TransactionType::InvokeFunction => {
             let execute_calldata = create_calldata(sender_address, "foo", &[]);
-            let invoke_tx = invoke_tx(invoke_tx_args! {
+            invoke_tx(invoke_tx_args! {
                 max_fee,
                 resource_bounds,
                 signature,
                 sender_address,
                 calldata: execute_calldata,
                 version: tx_version,
                 nonce: nonce_manager.next(sender_address),
-            });
-            AccountTransaction::Invoke(invoke_tx)
+            })
         }
         _ => panic!("{tx_type:?} is not an account transaction."),
     }
 }
 
 pub fn account_invoke_tx(invoke_args: InvokeTxArgs) -> AccountTransaction {
-    AccountTransaction::Invoke(invoke_tx(invoke_args))
+    invoke_tx(invoke_args)
 }
 
 pub fn run_invoke_tx(

crates/blockifier/src/transaction/transactions_test.rs

@@ -447,17 +447,17 @@ fn test_invoke_tx(
     let state = &mut test_state(chain_info, BALANCE, &[(account_contract, 1), (test_contract, 1)]);
     let test_contract_address = test_contract.get_instance_address(0);
     let account_contract_address = account_contract.get_instance_address(0);
+    let calldata = create_trivial_calldata(test_contract_address);
     let invoke_tx = invoke_tx(invoke_tx_args! {
         sender_address: account_contract_address,
-        calldata: create_trivial_calldata(test_contract_address),
+        calldata: Calldata(Arc::clone(&calldata.0)),
         resource_bounds,
     });
 
     // Extract invoke transaction fields for testing, as it is consumed when creating an account
     // transaction.
-    let calldata = Calldata(Arc::clone(&invoke_tx.calldata().0));
-    let calldata_length = invoke_tx.calldata().0.len();
-    let signature_length = invoke_tx.signature().0.len();
+    let calldata_length = invoke_tx.calldata_length();
+    let signature_length = invoke_tx.signature_length();
     let state_changes_for_fee = StateChangesCount {
         n_storage_updates: 1,
         n_modified_contracts: 1,
@@ -473,10 +473,9 @@ fn test_invoke_tx(
     );
     let sender_address = invoke_tx.sender_address();
 
-    let account_tx = AccountTransaction::Invoke(invoke_tx);
-    let tx_context = block_context.to_tx_context(&account_tx);
+    let tx_context = block_context.to_tx_context(&invoke_tx);
 
-    let actual_execution_info = account_tx.execute(state, block_context, true, true).unwrap();
+    let actual_execution_info = invoke_tx.execute(state, block_context, true, true).unwrap();
 
     let tracked_resource = account_contract.get_runnable_class().tracked_resource(
         &versioned_constants.min_compiler_version_for_sierra_gas,
@@ -967,13 +966,13 @@ fn test_max_fee_exceeds_balance(
     )};
 
     // Deploy.
-    let invalid_tx = AccountTransaction::DeployAccount(deploy_account_tx(
+    let invalid_tx = deploy_account_tx(
         deploy_account_tx_args! {
             resource_bounds,
             class_hash: test_contract.get_class_hash()
         },
         &mut NonceManager::default(),
-    ));
+    );
     assert_resource_bounds_exceed_balance_failure(state, block_context, invalid_tx);
 
     // V1 Invoke.
@@ -1703,10 +1702,8 @@ fn test_deploy_account_tx(
 
     // Extract deploy account transaction fields for testing, as it is consumed when creating an
     // account transaction.
-    let class_hash = deploy_account.class_hash();
-    let deployed_account_address = deploy_account.contract_address();
-    let constructor_calldata = deploy_account.constructor_calldata();
-    let salt = deploy_account.contract_address_salt();
+    let class_hash = deploy_account.class_hash().unwrap();
+    let deployed_account_address = deploy_account.sender_address();
     let user_initial_gas = user_initial_gas_from_bounds(default_all_resource_bounds);
 
     // Update the balance of the about to be deployed account contract in the erc20 contract, so it
@@ -1722,20 +1719,32 @@ fn test_deploy_account_tx(
             .unwrap();
     }
 
-    let account_tx = AccountTransaction::DeployAccount(deploy_account);
-    let fee_type = &account_tx.fee_type();
-    let tx_context = &block_context.to_tx_context(&account_tx);
-    let actual_execution_info = account_tx.execute(state, block_context, true, true).unwrap();
+    let fee_type = &deploy_account.fee_type();
+    let tx_context = &block_context.to_tx_context(&deploy_account);
+    let actual_execution_info = deploy_account.execute(state, block_context, true, true).unwrap();
 
     // Build expected validate call info.
-    let validate_calldata =
-        [vec![class_hash.0, salt.0], (*constructor_calldata.0).clone()].concat();
+    // TODO(AvivG): When the AccountTransaction refactor is complete, simplify the creation of
+    // `validate_calldata` by accessing the DeployAccountTransaction directly, without match.
+    let validate_calldata = match deploy_account {
+        AccountTransaction::DeployAccount(tx) => Calldata(
+            [
+                vec![tx.class_hash().0, tx.contract_address_salt().0],
+                (*tx.constructor_calldata().0).clone(),
+            ]
+            .concat()
+            .into(),
+        ),
+        AccountTransaction::Invoke(_) | AccountTransaction::Declare(_) => {
+            panic!("Expected DeployAccount transaction.")
+        }
+    };
     let expected_gas_consumed = 0;
     let expected_validate_call_info = expected_validate_call_info(
         account_class_hash,
         constants::VALIDATE_DEPLOY_ENTRY_POINT_NAME,
         expected_gas_consumed,
-        Calldata(validate_calldata.into()),
+        validate_calldata,
         deployed_account_address,
         cairo_version,
         account.get_runnable_class().tracked_resource(
@@ -1848,8 +1857,7 @@ fn test_deploy_account_tx(
         },
         &mut nonce_manager,
     );
-    let account_tx = AccountTransaction::DeployAccount(deploy_account);
-    let error = account_tx.execute(state, block_context, true, true).unwrap_err();
+    let error = deploy_account.execute(state, block_context, true, true).unwrap_err();
     assert_matches!(
         error,
         TransactionExecutionError::ContractConstructorExecutionFailed(
@@ -1886,13 +1894,12 @@ fn test_fail_deploy_account_undeclared_class_hash(
     state
         .set_storage_at(
             chain_info.fee_token_address(&fee_type),
-            get_fee_token_var_address(deploy_account.contract_address()),
+            get_fee_token_var_address(deploy_account.sender_address()),
             felt!(BALANCE.0),
         )
         .unwrap();
 
-    let account_tx = AccountTransaction::DeployAccount(deploy_account);
-    let error = account_tx.execute(state, block_context, true, true).unwrap_err();
+    let error = deploy_account.execute(state, block_context, true, true).unwrap_err();
     assert_matches!(
         error,
         TransactionExecutionError::ContractConstructorExecutionFailed(
@@ -2225,9 +2232,8 @@ fn test_only_query_flag(
         sender_address,
         only_query,
     });
-    let account_tx = AccountTransaction::Invoke(invoke_tx);
 
-    let tx_execution_info = account_tx.execute(state, block_context, true, true).unwrap();
+    let tx_execution_info = invoke_tx.execute(state, block_context, true, true).unwrap();
     assert_eq!(tx_execution_info.revert_error, None);
 }