From e4d8c1898253a3b89674d1e383dba12e8ab1e8ba Mon Sep 17 00:00:00 2001 From: Aner Ben Efraim Date: Mon, 16 Sep 2024 16:05:41 +0300 Subject: [PATCH] test(blockifier): max fee exceeds balance errors on new resource bounds --- crates/blockifier/src/fee/fee_utils.rs | 5 +- crates/blockifier/src/test_utils.rs | 3 + crates/blockifier/src/transaction/errors.rs | 4 +- .../src/transaction/transactions_test.rs | 91 ++++++++++++++++--- 4 files changed, 87 insertions(+), 16 deletions(-) diff --git a/crates/blockifier/src/fee/fee_utils.rs b/crates/blockifier/src/fee/fee_utils.rs index abcbaf05e9..756f46a928 100644 --- a/crates/blockifier/src/fee/fee_utils.rs +++ b/crates/blockifier/src/fee/fee_utils.rs @@ -125,12 +125,11 @@ pub fn verify_can_pay_committed_bounds( Fee(u128::from(l1_gas.max_amount) * l1_gas.max_price_per_unit) } // New resource bounds, also includes L1 Data Gas and L2 Gas. - // TODO!(Aner): add tests AllResources(AllResourceBounds { l1_gas, l2_gas, l1_data_gas }) => { // Committed fee is sum of products (resource_max_amount * resource_max_price) // of the different resources. - // The Sender will not be charged by`max_price_per_unit`, but this check should - // not depend on the current gas price + // The Sender will not be charged by `max_price_per_unit`, but this check should + // not depend on the current gas prices. Fee(u128::from(l1_gas.max_amount) * l1_gas.max_price_per_unit + u128::from(l1_data_gas.max_amount) * l1_data_gas.max_price_per_unit + u128::from(l2_gas.max_amount) * l2_gas.max_price_per_unit) diff --git a/crates/blockifier/src/test_utils.rs b/crates/blockifier/src/test_utils.rs index 55b8b79d4e..9463fc588b 100644 --- a/crates/blockifier/src/test_utils.rs +++ b/crates/blockifier/src/test_utils.rs @@ -98,6 +98,8 @@ pub const MAX_L1_GAS_AMOUNT: u64 = 1000000; #[allow(clippy::as_conversions)] pub const MAX_L1_GAS_AMOUNT_U128: u128 = MAX_L1_GAS_AMOUNT as u128; pub const MAX_L1_GAS_PRICE: u128 = DEFAULT_STRK_L1_GAS_PRICE; +pub const MAX_L2_GAS_PRICE: u128 = DEFAULT_STRK_L2_GAS_PRICE; +pub const MAX_L1_DATA_GAS_PRICE: u128 = DEFAULT_STRK_L1_DATA_GAS_PRICE; pub const MAX_RESOURCE_COMMITMENT: u128 = MAX_L1_GAS_AMOUNT_U128 * MAX_L1_GAS_PRICE; pub const MAX_FEE: u128 = MAX_L1_GAS_AMOUNT_U128 * DEFAULT_ETH_L1_GAS_PRICE; @@ -107,6 +109,7 @@ pub const BALANCE: u128 = 10 * const_max(MAX_FEE, MAX_RESOURCE_COMMITMENT); pub const DEFAULT_ETH_L1_GAS_PRICE: u128 = 100 * u128::pow(10, 9); // Given in units of Wei. pub const DEFAULT_STRK_L1_GAS_PRICE: u128 = 100 * u128::pow(10, 9); // Given in units of STRK. +pub const DEFAULT_STRK_L2_GAS_PRICE: u128 = 4000000 * u128::pow(10, 9); // Given in units of STRK. pub const DEFAULT_ETH_L1_DATA_GAS_PRICE: u128 = u128::pow(10, 6); // Given in units of Wei. pub const DEFAULT_STRK_L1_DATA_GAS_PRICE: u128 = u128::pow(10, 9); // Given in units of STRK. diff --git a/crates/blockifier/src/transaction/errors.rs b/crates/blockifier/src/transaction/errors.rs index 3e73d7a881..ee9b870db5 100644 --- a/crates/blockifier/src/transaction/errors.rs +++ b/crates/blockifier/src/transaction/errors.rs @@ -39,8 +39,8 @@ pub enum TransactionFeeError { balance: BigUint, }, #[error( - "Resource {resource} Gas bounds (max amount: {max_amount}, max price): {max_price}) \ - exceed balance ({balance})." + "Resource {resource} Gas bounds (max amount: {max_amount}, max price: {max_price}) exceed \ + balance ({balance})." )] GasBoundsExceedBalance { resource: Resource, diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index 7b5787137d..8c4c61e486 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -92,8 +92,10 @@ use crate::test_utils::{ CURRENT_BLOCK_TIMESTAMP, CURRENT_BLOCK_TIMESTAMP_FOR_VALIDATE, MAX_FEE, + MAX_L1_DATA_GAS_PRICE, MAX_L1_GAS_AMOUNT, MAX_L1_GAS_PRICE, + MAX_L2_GAS_PRICE, TEST_SEQUENCER_ADDRESS, }; use crate::transaction::account_transaction::AccountTransaction; @@ -816,16 +818,39 @@ fn assert_failure_if_resource_bounds_exceed_balance( if max_fee == context.max_fee ); } - TransactionInfo::Current(context) => { - let l1_bounds = context.l1_resource_bounds(); - assert_matches!( + TransactionInfo::Current(context) => match &context.resource_bounds { + ValidResourceBounds::L1Gas(l1_bounds) => assert_matches!( invalid_tx.execute(state, block_context, true, true).unwrap_err(), TransactionExecutionError::TransactionPreValidationError( TransactionPreValidationError::TransactionFeeError( - TransactionFeeError::GasBoundsExceedBalance{resource, max_amount, max_price, .. })) + 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: l1_bounds, + l2_gas: l2_bounds, + l1_data_gas: l1_data_bounds, + }) => { + assert_matches!( + invalid_tx.execute(state, block_context, true, true).unwrap_err(), + TransactionExecutionError::TransactionPreValidationError( + TransactionPreValidationError::TransactionFeeError( + TransactionFeeError::ResourcesBoundsExceedBalance{ + l1_max_amount, + l1_max_price, + l1_data_max_amount, + l1_data_max_price, + l2_max_amount, + l2_max_price, .. } + ) + ) + if l1_max_amount == l1_bounds.max_amount && l1_max_price == l1_bounds.max_price_per_unit + && l2_max_amount == l2_bounds.max_amount && l2_max_price == l2_bounds.max_price_per_unit + && l1_data_max_amount == l1_data_bounds.max_amount + && l1_data_max_price == l1_data_bounds.max_price_per_unit + ); + } + }, }; } @@ -884,9 +909,11 @@ fn test_max_fee_exceeds_balance( let invalid_max_fee = Fee(BALANCE + 1); // TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the conversion works. - let balance_over_gas_price: u64 = + let balance_over_l1_gas_price: u64 = (BALANCE / MAX_L1_GAS_PRICE).try_into().expect("Failed to convert u128 to u64."); - let invalid_resource_bounds = l1_resource_bounds(balance_over_gas_price + 1, MAX_L1_GAS_PRICE); + + let invalid_l1_resource_bounds = + l1_resource_bounds(balance_over_l1_gas_price + 1, MAX_L1_GAS_PRICE); // V1 Invoke. let invalid_tx = account_invoke_tx(invoke_tx_args! { @@ -898,8 +925,8 @@ fn test_max_fee_exceeds_balance( // V3 invoke. let invalid_tx = account_invoke_tx(invoke_tx_args! { - resource_bounds: invalid_resource_bounds.clone(), - ..default_args + resource_bounds: invalid_l1_resource_bounds.clone(), + ..default_args.clone() }); assert_failure_if_resource_bounds_exceed_balance(state, block_context, invalid_tx); @@ -921,11 +948,53 @@ fn test_max_fee_exceeds_balance( 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, + resource_bounds: invalid_l1_resource_bounds.clone(), }, class_info, ); assert_failure_if_resource_bounds_exceed_balance(state, block_context, invalid_tx); + + // TODO!(Aner): test also with use_kzg_da == true + // Test all resource bounds. + // TODO!(Aner): remove magic numbers, use function to calculate minimal amounts. + let min_l1_amount = 1652; + let min_l2_amount = 439500; + let balance_over_l2_gas_price: u64 = + (BALANCE / MAX_L2_GAS_PRICE).try_into().expect("Failed to convert u128 to u64."); + let balance_over_l1_data_gas_price: u64 = + (BALANCE / MAX_L1_DATA_GAS_PRICE).try_into().expect("Failed to convert u128 to u64."); + // In the following cases, the balance is not enough to cover the gas price of the transaction. + // Minimal amounts are used to avoid failing the test due to min gas usage not covered. + for (l1_max_amount, l2_max_amount, l1_data_max_amount) in [ + (min_l1_amount + balance_over_l1_gas_price + 1, min_l2_amount, 0), + (min_l1_amount, min_l2_amount + balance_over_l2_gas_price + 1, 0), + (min_l1_amount, min_l2_amount, balance_over_l1_data_gas_price + 1), + ( + min_l1_amount + balance_over_l1_gas_price / 3 + 1, + min_l2_amount + balance_over_l2_gas_price / 3 + 1, + balance_over_l1_data_gas_price / 3 + 1, + ), + ] { + let invalid_resource_bounds = ValidResourceBounds::AllResources(AllResourceBounds { + l1_gas: ResourceBounds { + max_amount: l1_max_amount, + max_price_per_unit: MAX_L1_GAS_PRICE, + }, + l2_gas: ResourceBounds { + max_amount: l2_max_amount, + max_price_per_unit: MAX_L2_GAS_PRICE, + }, + l1_data_gas: ResourceBounds { + max_amount: l1_data_max_amount, + max_price_per_unit: MAX_L1_DATA_GAS_PRICE, + }, + }); + 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); + } } #[rstest]