From bac51a07f79e5f51dbe7a6db79e9ccc60c86f875 Mon Sep 17 00:00:00 2001 From: Aviv Greenburg Date: Tue, 3 Dec 2024 14:05:20 +0200 Subject: [PATCH] chore(blockifier): use test_utils::invoke_tx() instead of trans::test_utils::account_invoke_tx() --- .../blockifier/transaction_executor_test.rs | 8 +- .../transaction/account_transactions_test.rs | 9 +- .../src/transaction/execution_flavors_test.rs | 107 +++++++++++------- .../blockifier/src/transaction/test_utils.rs | 14 ++- .../src/transaction/transactions_test.rs | 3 +- 5 files changed, 85 insertions(+), 56 deletions(-) diff --git a/crates/blockifier/src/blockifier/transaction_executor_test.rs b/crates/blockifier/src/blockifier/transaction_executor_test.rs index 8deac77a25..0816316998 100644 --- a/crates/blockifier/src/blockifier/transaction_executor_test.rs +++ b/crates/blockifier/src/blockifier/transaction_executor_test.rs @@ -21,6 +21,7 @@ use crate::test_utils::contracts::FeatureContract; use crate::test_utils::declare::declare_tx; use crate::test_utils::deploy_account::deploy_account_tx; use crate::test_utils::initial_test_state::test_state; +use crate::test_utils::invoke::invoke_tx; use crate::test_utils::l1_handler::l1handler_tx; use crate::test_utils::{ create_calldata, @@ -32,7 +33,6 @@ use crate::test_utils::{ use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::errors::TransactionExecutionError; use crate::transaction::test_utils::{ - account_invoke_tx, block_context, calculate_class_info_for_testing, create_test_init_data, @@ -218,12 +218,12 @@ fn test_invoke( let calldata = create_calldata(test_contract.get_instance_address(0), entry_point_name, &entry_point_args); - let tx = account_invoke_tx(invoke_tx_args! { + let invoke_tx = invoke_tx(invoke_tx_args! { sender_address: account_contract.get_instance_address(0), calldata, version, - }) - .into(); + }); + let tx = AccountTransaction { tx: invoke_tx, only_query: false }.into(); tx_executor_test_body(state, block_context, tx, expected_bouncer_weights); } diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index d070ca9fa3..a016105508 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -66,6 +66,7 @@ use crate::test_utils::contracts::FeatureContract; use crate::test_utils::declare::declare_tx; use crate::test_utils::deploy_account::deploy_account_tx; use crate::test_utils::initial_test_state::{fund_account, test_state}; +use crate::test_utils::invoke::invoke_tx; use crate::test_utils::syscall::build_recurse_calldata; use crate::test_utils::{ create_calldata, @@ -1758,12 +1759,12 @@ fn test_revert_in_execute( // Skip validate phase, as we want to test the revert in the execute phase. let validate = false; - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { resource_bounds: default_all_resource_bounds, ..tx_args - }) - .execute(state, &block_context, true, validate) - .unwrap(); + }); + let account_tx = AccountTransaction { tx, only_query: false }; + let tx_execution_info = account_tx.execute(state, &block_context, true, validate).unwrap(); assert!(tx_execution_info.is_reverted()); assert!( diff --git a/crates/blockifier/src/transaction/execution_flavors_test.rs b/crates/blockifier/src/transaction/execution_flavors_test.rs index 9358e6d2e4..202197cc12 100644 --- a/crates/blockifier/src/transaction/execution_flavors_test.rs +++ b/crates/blockifier/src/transaction/execution_flavors_test.rs @@ -26,6 +26,7 @@ use crate::state::state_api::StateReader; use crate::test_utils::contracts::FeatureContract; use crate::test_utils::dict_state_reader::DictStateReader; use crate::test_utils::initial_test_state::test_state; +use crate::test_utils::invoke::invoke_tx; use crate::test_utils::{ create_calldata, create_trivial_calldata, @@ -37,6 +38,7 @@ use crate::test_utils::{ DEFAULT_STRK_L1_GAS_PRICE, MAX_FEE, }; +use crate::transaction::account_transaction::AccountTransaction; use crate::transaction::errors::{ TransactionExecutionError, TransactionFeeError, @@ -223,9 +225,9 @@ fn test_invalid_nonce_pre_validate( // First scenario: invalid nonce. Regardless of flags, should fail. let invalid_nonce = nonce!(7_u8); let account_nonce = state.get_nonce_at(account_address).unwrap(); - let result = - account_invoke_tx(invoke_tx_args! {nonce: invalid_nonce, ..pre_validation_base_args}) - .execute(&mut state, &block_context, charge_fee, validate); + let tx = invoke_tx(invoke_tx_args! {nonce: invalid_nonce, ..pre_validation_base_args}); + let account_tx = AccountTransaction { tx, only_query }; + let result = account_tx.execute(&mut state, &block_context, charge_fee, validate); assert_matches!( result.unwrap_err(), TransactionExecutionError::TransactionPreValidationError( @@ -295,16 +297,18 @@ fn test_simulate_validate_pre_validate_with_charge_fee( // Second scenario: resource bounds greater than balance. let gas_price = block_context.block_info.gas_prices.l1_gas_price(&fee_type); let balance_over_gas_price = BALANCE.checked_div(gas_price).unwrap(); - let result = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee: Fee(BALANCE.0 + 1), resource_bounds: l1_resource_bounds( (balance_over_gas_price.0 + 10).into(), gas_price.into() ), nonce: nonce_manager.next(account_address), + ..pre_validation_base_args.clone() - }) - .execute(&mut state, &block_context, charge_fee, validate); + }); + let account_tx = AccountTransaction { tx, only_query }; + let result = account_tx.execute(&mut state, &block_context, charge_fee, validate); nonce_manager.rollback(account_address); if is_deprecated { @@ -330,13 +334,14 @@ fn test_simulate_validate_pre_validate_with_charge_fee( // Third scenario: L1 gas price bound lower than the price on the block. if !is_deprecated { - let err = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { resource_bounds: l1_resource_bounds(DEFAULT_L1_GAS_AMOUNT, (gas_price.get().0 - 1).into()), nonce: nonce_manager.next(account_address), + ..pre_validation_base_args - }) - .execute(&mut state, &block_context, charge_fee, validate) - .unwrap_err(); + }); + let account_tx = AccountTransaction { tx, only_query }; + let err = account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap_err(); nonce_manager.rollback(account_address); assert_matches!( @@ -369,12 +374,14 @@ fn test_simulate_validate_pre_validate_not_charge_fee( get_pre_validate_test_args(cairo_version, version, only_query); let account_address = pre_validation_base_args.sender_address; - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { nonce: nonce_manager.next(account_address), ..pre_validation_base_args.clone() - }) - .execute(&mut state, &block_context, charge_fee, false) - .unwrap(); + }); + let account_tx = AccountTransaction { tx, only_query }; + let tx_execution_info = + account_tx.execute(&mut state, &block_context, charge_fee, false).unwrap(); + let base_gas = calculate_actual_gas(&tx_execution_info, &block_context, false); assert!( base_gas @@ -388,14 +395,17 @@ fn test_simulate_validate_pre_validate_not_charge_fee( let (actual_gas_used, actual_fee) = gas_and_fee(base_gas, validate, &fee_type); macro_rules! execute_and_check_gas_and_fee { ($max_fee:expr, $resource_bounds:expr) => {{ - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee: $max_fee, resource_bounds: $resource_bounds, nonce: nonce_manager.next(account_address), + ..pre_validation_base_args.clone() - }) - .execute(&mut state, &block_context, charge_fee, validate) - .unwrap(); + }); + let account_tx = AccountTransaction { tx, only_query }; + let tx_execution_info = + account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap(); + check_gas_and_fee( &block_context, &tx_execution_info, @@ -448,7 +458,7 @@ fn execute_fail_validation( } = create_flavors_test_state(&block_context.chain_info, cairo_version); // Validation scenario: fallible validation. - account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee, resource_bounds: max_resource_bounds, signature: TransactionSignature(vec![ @@ -459,9 +469,9 @@ fn execute_fail_validation( calldata: create_calldata(faulty_account_address, "foo", &[]), version, nonce: nonce_manager.next(faulty_account_address), - only_query, - }) - .execute(&mut falliable_state, &block_context, charge_fee, validate) + }); + let account_tx = AccountTransaction { tx, only_query }; + account_tx.execute(&mut falliable_state, &block_context, charge_fee, validate) } /// Test simulate / charge_fee flag combinations in (fallible) validation stage. @@ -577,13 +587,15 @@ fn test_simulate_validate_charge_fee_mid_execution( }; // First scenario: logic error. Should result in revert; actual fee should be shown. - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { calldata: recurse_calldata(test_contract_address, true, 3), nonce: nonce_manager.next(account_address), + only_query, ..execution_base_args.clone() - }) - .execute(&mut state, &block_context, charge_fee, validate) - .unwrap(); + }); + let account_tx = AccountTransaction { tx, only_query }; + let tx_execution_info = + account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap(); let base_gas = calculate_actual_gas(&tx_execution_info, &block_context, validate); let (revert_gas_used, revert_fee) = gas_and_fee(base_gas, validate, &fee_type); assert!( @@ -623,15 +635,19 @@ fn test_simulate_validate_charge_fee_mid_execution( validate, &fee_type, ); - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee: fee_bound, resource_bounds: l1_resource_bounds(gas_bound, gas_price.into()), calldata: recurse_calldata(test_contract_address, false, 1000), nonce: nonce_manager.next(account_address), + only_query, + ..execution_base_args.clone() - }) - .execute(&mut state, &block_context, charge_fee, validate) - .unwrap(); + }); + let account_tx = AccountTransaction { tx, only_query }; + let tx_execution_info = + account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap(); + assert_eq!(tx_execution_info.is_reverted(), charge_fee); if charge_fee { assert!( @@ -676,15 +692,17 @@ fn test_simulate_validate_charge_fee_mid_execution( GasVector::from_l1_gas(block_limit_gas), &fee_type, ); - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee: huge_fee, resource_bounds: l1_resource_bounds(huge_gas_limit, gas_price.into()), calldata: recurse_calldata(test_contract_address, false, 10000), nonce: nonce_manager.next(account_address), ..execution_base_args - }) - .execute(&mut state, &low_step_block_context, charge_fee, validate) - .unwrap(); + }); + let account_tx = AccountTransaction { tx, only_query }; + let tx_execution_info = + account_tx.execute(&mut state, &low_step_block_context, charge_fee, validate).unwrap(); + assert!( tx_execution_info.revert_error.clone().unwrap().to_string().contains("no remaining steps") ); @@ -761,7 +779,7 @@ fn test_simulate_validate_charge_fee_post_execution( validate, &fee_type, ); - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee: just_not_enough_fee_bound, resource_bounds: l1_resource_bounds(just_not_enough_gas_bound, gas_price.into()), calldata: recurse_calldata(test_contract_address, false, 1000), @@ -769,9 +787,11 @@ fn test_simulate_validate_charge_fee_post_execution( sender_address: account_address, version, only_query, - }) - .execute(&mut state, &block_context, charge_fee, validate) - .unwrap(); + }); + let account_tx = AccountTransaction { tx, only_query }; + let tx_execution_info = + account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap(); + assert_eq!(tx_execution_info.is_reverted(), charge_fee); if charge_fee { let expected_error_prefix = @@ -821,7 +841,7 @@ fn test_simulate_validate_charge_fee_post_execution( felt!(0_u8), ], ); - let tx_execution_info = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { max_fee: actual_fee, resource_bounds: l1_resource_bounds(success_actual_gas, gas_price.into()), calldata: transfer_calldata, @@ -829,9 +849,12 @@ fn test_simulate_validate_charge_fee_post_execution( sender_address: account_address, version, only_query, - }) - .execute(&mut state, &block_context, charge_fee, validate) - .unwrap(); + + }); + let account_tx = AccountTransaction { tx, only_query }; + let tx_execution_info = + account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap(); + assert_eq!(tx_execution_info.is_reverted(), charge_fee); if charge_fee { diff --git a/crates/blockifier/src/transaction/test_utils.rs b/crates/blockifier/src/transaction/test_utils.rs index 68a2b98936..beff26d504 100644 --- a/crates/blockifier/src/transaction/test_utils.rs +++ b/crates/blockifier/src/transaction/test_utils.rs @@ -301,10 +301,12 @@ pub fn run_invoke_tx( block_context: &BlockContext, invoke_args: InvokeTxArgs, ) -> TransactionExecutionResult { - let tx = account_invoke_tx(invoke_args); - let charge_fee = tx.enforce_fee(); + let only_query = invoke_args.only_query; + let tx = invoke_tx(invoke_args); + let account_tx = AccountTransaction { tx, only_query }; + let charge_fee = account_tx.enforce_fee(); - tx.execute(state, block_context, charge_fee, true) + account_tx.execute(state, block_context, charge_fee, true) } /// Creates a `ResourceBoundsMapping` with the given `max_amount` and `max_price` for L1 gas limits. @@ -374,9 +376,11 @@ pub fn emit_n_events_tx( felt!(0_u32), // data length. ]; let calldata = create_calldata(contract_address, "test_emit_events", &entry_point_args); - account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { sender_address: account_contract, calldata, nonce - }) + }); + + AccountTransaction { tx, only_query: false } } diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index f6fb7c0388..a5d8511b2a 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -2147,11 +2147,12 @@ fn test_valid_flag( &[(account_contract, 1), (test_contract, 1)], ); - let account_tx = account_invoke_tx(invoke_tx_args! { + let tx = invoke_tx(invoke_tx_args! { sender_address: account_contract.get_instance_address(0), calldata: create_trivial_calldata(test_contract.get_instance_address(0)), resource_bounds: default_all_resource_bounds, }); + let account_tx = AccountTransaction { tx, only_query: false }; let actual_execution_info = account_tx.execute(state, block_context, true, false).unwrap();