From 8b2feeb66fbe27e8305423f60504c30289e84bb8 Mon Sep 17 00:00:00 2001 From: Dori Medini Date: Fri, 11 Oct 2024 13:23:16 +0300 Subject: [PATCH] test(blockifier): parametrize test_max_fee_exceeds_balance by resource bounds types --- .../src/transaction/transactions_test.rs | 192 ++++++++++++++---- 1 file changed, 147 insertions(+), 45 deletions(-) diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index ce3ebacd7fe..513d942b887 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -104,7 +104,9 @@ use crate::test_utils::{ CURRENT_BLOCK_TIMESTAMP, CURRENT_BLOCK_TIMESTAMP_FOR_VALIDATE, DEFAULT_L1_GAS_AMOUNT, + DEFAULT_STRK_L1_DATA_GAS_PRICE, DEFAULT_STRK_L1_GAS_PRICE, + DEFAULT_STRK_L2_GAS_PRICE, MAX_FEE, TEST_SEQUENCER_ADDRESS, }; @@ -127,6 +129,7 @@ use crate::transaction::test_utils::{ calculate_class_info_for_testing, create_account_tx_for_validate_test, create_account_tx_for_validate_test_nonce_0, + create_all_resource_bounds, default_all_resource_bounds, default_l1_resource_bounds, l1_resource_bounds, @@ -609,7 +612,7 @@ fn verify_storage_after_invoke_advanced_operations( #[rstest] fn test_invoke_tx_advanced_operations( block_context: BlockContext, - default_l1_resource_bounds: ValidResourceBounds, + default_all_resource_bounds: ValidResourceBounds, #[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion, ) { let block_context = &block_context; @@ -621,7 +624,7 @@ fn test_invoke_tx_advanced_operations( let contract_address = test_contract.get_instance_address(0); let index = felt!(123_u32); let base_tx_args = invoke_tx_args! { - resource_bounds: default_l1_resource_bounds, + resource_bounds: default_all_resource_bounds, sender_address: account_address, }; @@ -825,26 +828,61 @@ fn assert_failure_if_resource_bounds_exceed_balance( block_context: &BlockContext, invalid_tx: AccountTransaction, ) { + let tx_error = invalid_tx.execute(state, block_context, true, true).unwrap_err(); match block_context.to_tx_context(&invalid_tx).tx_info { TransactionInfo::Deprecated(context) => { assert_matches!( - invalid_tx.execute(state, block_context, true, true).unwrap_err(), + tx_error, TransactionExecutionError::TransactionPreValidationError( TransactionPreValidationError::TransactionFeeError( TransactionFeeError::MaxFeeExceedsBalance{ max_fee, .. })) if max_fee == context.max_fee ); } - TransactionInfo::Current(context) => { - let l1_bounds = context.l1_resource_bounds(); - assert_matches!( - invalid_tx.execute(state, block_context, true, true).unwrap_err(), + TransactionInfo::Current(context) => match context.resource_bounds { + ValidResourceBounds::L1Gas(l1_bounds) => assert_matches!( + tx_error, TransactionExecutionError::TransactionPreValidationError( TransactionPreValidationError::TransactionFeeError( - TransactionFeeError::GasBoundsExceedBalance{resource, max_amount, max_price, .. })) - if max_amount == l1_bounds.max_amount && max_price == l1_bounds.max_price_per_unit && resource == L1Gas - ); - } + TransactionFeeError::GasBoundsExceedBalance{ + resource, max_amount, max_price, .. + } + ) + ) + if max_amount == l1_bounds.max_amount + && max_price == l1_bounds.max_price_per_unit + && resource == L1Gas + ), + ValidResourceBounds::AllResources(AllResourceBounds { + l1_gas: ResourceBounds { max_amount: l1_gas, max_price_per_unit: l1_gas_price }, + l2_gas: ResourceBounds { max_amount: l2_gas, max_price_per_unit: l2_gas_price }, + l1_data_gas: + ResourceBounds { max_amount: l1_data_gas, max_price_per_unit: l1_data_gas_price }, + }) => { + assert_matches!( + tx_error, + TransactionExecutionError::TransactionPreValidationError( + TransactionPreValidationError::TransactionFeeError( + TransactionFeeError::ResourcesBoundsExceedBalance { + l1_max_amount, + l1_max_price, + l2_max_amount, + l2_max_price, + l1_data_max_amount, + l1_data_max_price, + .. + } + ) + ) + if l1_max_amount == l1_gas + && l1_max_price == l1_gas_price + && l2_max_amount == l2_gas + && l2_max_price == l2_gas_price + && l1_data_max_amount == l1_data_gas + && l1_data_max_price == l1_data_gas_price + ); + } + }, }; } @@ -887,7 +925,8 @@ fn test_estimate_minimal_gas_vector( #[rstest] fn test_max_fee_exceeds_balance( block_context: BlockContext, - default_l1_resource_bounds: ValidResourceBounds, + #[values(default_l1_resource_bounds(), default_all_resource_bounds())] + resource_bounds: ValidResourceBounds, #[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] account_cairo_version: CairoVersion, ) { let block_context = &block_context; @@ -903,51 +942,114 @@ fn test_max_fee_exceeds_balance( sender_address: account_contract_address, calldata: create_trivial_calldata(test_contract.get_instance_address(0) )}; - let invalid_max_fee = Fee(BALANCE.0 + 1); - // TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the conversion works. - let balance_over_gas_price = BALANCE.checked_div(DEFAULT_STRK_L1_GAS_PRICE).unwrap(); - let invalid_resource_bounds = - l1_resource_bounds((balance_over_gas_price.0 + 1).into(), DEFAULT_STRK_L1_GAS_PRICE.into()); - - // V1 Invoke. - let invalid_tx = account_invoke_tx(invoke_tx_args! { - max_fee: invalid_max_fee, - version: TransactionVersion::ONE, - ..default_args.clone() - }); - assert_failure_if_resource_bounds_exceed_balance(state, block_context, invalid_tx); - - // V3 invoke. - let invalid_tx = account_invoke_tx(invoke_tx_args! { - resource_bounds: invalid_resource_bounds, - ..default_args - }); - assert_failure_if_resource_bounds_exceed_balance(state, block_context, invalid_tx); // Deploy. let invalid_tx = AccountTransaction::DeployAccount(deploy_account_tx( deploy_account_tx_args! { - resource_bounds: default_l1_resource_bounds, + resource_bounds, class_hash: test_contract.get_class_hash() }, &mut NonceManager::default(), )); assert_failure_if_resource_bounds_exceed_balance(state, block_context, invalid_tx); - // Declare. - let contract_to_declare = FeatureContract::Empty(CairoVersion::Cairo1); - let class_info = calculate_class_info_for_testing(contract_to_declare.get_class()); - let invalid_tx = declare_tx( - declare_tx_args! { - class_hash: contract_to_declare.get_class_hash(), - compiled_class_hash: contract_to_declare.get_compiled_class_hash(), - sender_address: account_contract_address, - resource_bounds: invalid_resource_bounds, - }, - class_info, - ); + // V1 Invoke. + let invalid_tx = account_invoke_tx(invoke_tx_args! { + max_fee: invalid_max_fee, + version: TransactionVersion::ONE, + ..default_args.clone() + }); assert_failure_if_resource_bounds_exceed_balance(state, block_context, invalid_tx); + + // V3 txs. + macro_rules! v3_txs_invalid_bounds_invoke_case { + ($invalid_resource_bounds:expr) => { + // V3 invoke. + let invalid_tx = account_invoke_tx(invoke_tx_args! { + resource_bounds: $invalid_resource_bounds, + ..default_args.clone() + }); + assert_failure_if_resource_bounds_exceed_balance(state, block_context, invalid_tx); + }; + } + macro_rules! v3_txs_invalid_bounds_declare_case { + ($invalid_resource_bounds:expr) => { + // Declare. + let contract_to_declare = FeatureContract::Empty(CairoVersion::Cairo1); + let class_info = calculate_class_info_for_testing(contract_to_declare.get_class()); + let invalid_tx = declare_tx( + declare_tx_args! { + class_hash: contract_to_declare.get_class_hash(), + compiled_class_hash: contract_to_declare.get_compiled_class_hash(), + sender_address: account_contract_address, + resource_bounds: $invalid_resource_bounds, + }, + class_info, + ); + assert_failure_if_resource_bounds_exceed_balance(state, block_context, invalid_tx); + }; + } + + // Test V3 insufficient balance w.r.t. the bounds type. + match resource_bounds.get_gas_vector_computation_mode() { + GasVectorComputationMode::NoL2Gas => { + let balance_over_l1_gas_price = BALANCE.checked_div(DEFAULT_STRK_L1_GAS_PRICE).unwrap(); + let invalid_resource_bounds = l1_resource_bounds( + (balance_over_l1_gas_price.0 + 1).into(), + DEFAULT_STRK_L1_GAS_PRICE.into(), + ); + v3_txs_invalid_bounds_invoke_case!(invalid_resource_bounds); + v3_txs_invalid_bounds_declare_case!(invalid_resource_bounds); + } + GasVectorComputationMode::All => { + // Divide balance into 3 parts, one for each resource. Get overdraft on each. + let partial_balance = Fee(BALANCE.0 / 3); + let l1_gas_amount = partial_balance.checked_div(DEFAULT_STRK_L1_GAS_PRICE).unwrap(); + let l2_gas_amount = partial_balance.checked_div(DEFAULT_STRK_L2_GAS_PRICE).unwrap(); + let l1_data_gas_amount = + partial_balance.checked_div(DEFAULT_STRK_L1_DATA_GAS_PRICE).unwrap(); + let ValidResourceBounds::AllResources(mut base_resource_bounds) = + create_all_resource_bounds( + l1_gas_amount, + DEFAULT_STRK_L1_GAS_PRICE.into(), + l2_gas_amount, + DEFAULT_STRK_L2_GAS_PRICE.into(), + l1_data_gas_amount, + DEFAULT_STRK_L1_DATA_GAS_PRICE.into(), + ) + else { + panic!("Invalid resource bounds."); + }; + // L1 gas overdraft. + base_resource_bounds.l1_gas.max_amount.0 += 10; + v3_txs_invalid_bounds_invoke_case!(ValidResourceBounds::AllResources( + base_resource_bounds + )); + v3_txs_invalid_bounds_declare_case!(ValidResourceBounds::AllResources( + base_resource_bounds + )); + base_resource_bounds.l1_gas.max_amount.0 -= 10; + // L2 gas overdraft. + base_resource_bounds.l2_gas.max_amount.0 += 10; + v3_txs_invalid_bounds_invoke_case!(ValidResourceBounds::AllResources( + base_resource_bounds + )); + v3_txs_invalid_bounds_declare_case!(ValidResourceBounds::AllResources( + base_resource_bounds + )); + base_resource_bounds.l2_gas.max_amount.0 -= 10; + // L1 data gas overdraft. + base_resource_bounds.l1_data_gas.max_amount.0 += 10; + v3_txs_invalid_bounds_invoke_case!(ValidResourceBounds::AllResources( + base_resource_bounds + )); + v3_txs_invalid_bounds_declare_case!(ValidResourceBounds::AllResources( + base_resource_bounds + )); + base_resource_bounds.l1_data_gas.max_amount.0 -= 10; + } + } } #[rstest]