Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(blockifier): max fee exceeds balance errors on new resource bounds #764

Open
wants to merge 1 commit into
base: aner/pre_validation_covers_sum_of_products_1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions crates/blockifier/src/fee/fee_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,11 @@ pub fn verify_can_pay_committed_bounds(
{
Fee(u128::from(l1_gas.max_amount) * l1_gas.max_price_per_unit)
}
// 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)
Expand Down
2 changes: 2 additions & 0 deletions crates/blockifier/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,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;

Expand Down
90 changes: 78 additions & 12 deletions crates/blockifier/src/transaction/transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,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;
Expand Down Expand Up @@ -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
);
}
},
};
}

Expand Down Expand Up @@ -864,10 +889,12 @@ fn test_estimate_minimal_gas_vector(

#[rstest]
fn test_max_fee_exceeds_balance(
block_context: BlockContext,
mut block_context: BlockContext,
max_resource_bounds: ValidResourceBounds,
#[values(true, false)] use_kzg_da: bool,
#[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] account_cairo_version: CairoVersion,
) {
block_context.block_info.use_kzg_da = use_kzg_da;
let block_context = &block_context;
let account_contract = FeatureContract::AccountWithoutValidations(account_cairo_version);
let test_contract = FeatureContract::TestContract(CairoVersion::Cairo0);
Expand All @@ -884,9 +911,10 @@ 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 max_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(max_l1_gas_price + 1, MAX_L1_GAS_PRICE);

// V1 Invoke.
let invalid_tx = account_invoke_tx(invoke_tx_args! {
Expand All @@ -898,8 +926,8 @@ fn test_max_fee_exceeds_balance(

// V3 invoke.
let invalid_tx = account_invoke_tx(invoke_tx_args! {
resource_bounds: invalid_resource_bounds,
..default_args
resource_bounds: invalid_l1_resource_bounds,
..default_args.clone()
});
assert_failure_if_resource_bounds_exceed_balance(state, block_context, invalid_tx);

Expand All @@ -921,11 +949,49 @@ 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,
},
class_info,
);
assert_failure_if_resource_bounds_exceed_balance(state, block_context, invalid_tx);

// Test all resource bounds.
// TODO!(Aner): remove magic numbers, use function to calculate minimal amounts.
let min_l1_amount = 1652;
let min_l2_amount = 445500;
let min_l1_data_amount = 128;
let max_l2_gas_price: u64 =
(BALANCE / MAX_L2_GAS_PRICE).try_into().expect("Failed to convert u128 to u64.");
let max_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 [
(max_l1_gas_price + 1, min_l2_amount, min_l1_data_amount),
(min_l1_amount, max_l2_gas_price + 1, min_l1_data_amount),
(min_l1_amount, min_l2_amount, max_l1_data_gas_price + 1),
(max_l1_gas_price / 3 + 1, max_l2_gas_price / 3 + 1, max_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]
Expand Down
Loading