diff --git a/crates/blockifier/src/concurrency/versioned_state_test.rs b/crates/blockifier/src/concurrency/versioned_state_test.rs index 261e175f59..f78ea7a2b9 100644 --- a/crates/blockifier/src/concurrency/versioned_state_test.rs +++ b/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( diff --git a/crates/blockifier/src/test_utils/deploy_account.rs b/crates/blockifier/src/test_utils/deploy_account.rs index d6745d46f2..51fd5a59f1 100644 --- a/crates/blockifier/src/test_utils/deploy_account.rs +++ b/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) } diff --git a/crates/blockifier/src/test_utils/invoke.rs b/crates/blockifier/src/test_utils/invoke.rs index 65f032d8a2..d4fff4923b 100644 --- a/crates/blockifier/src/test_utils/invoke.rs +++ b/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) } diff --git a/crates/blockifier/src/test_utils/transfers_generator.rs b/crates/blockifier/src/test_utils/transfers_generator.rs index 77f26a6c28..aa14add889 100644 --- a/crates/blockifier/src/test_utils/transfers_generator.rs +++ b/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) + }) } } diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 09335d4fa2..ef64b00115 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/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(), }; diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 3d39a48b54..abe7d11f30 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/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); diff --git a/crates/blockifier/src/transaction/test_utils.rs b/crates/blockifier/src/transaction/test_utils.rs index a2934a2206..0722c39268 100644 --- a/crates/blockifier/src/transaction/test_utils.rs +++ b/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,12 +292,11 @@ 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, @@ -306,15 +304,14 @@ pub fn create_account_tx_for_validate_test( 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( diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 956c228b65..323ad41c43 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/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); }