Skip to content

Commit

Permalink
test(blockifier): max fee exceeds balance errors on new resource bounds
Browse files Browse the repository at this point in the history
  • Loading branch information
aner-starkware committed Sep 24, 2024
1 parent 9095f45 commit 343c018
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 16 deletions.
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)
}
// 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)
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 @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/transaction/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,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,
Expand Down
91 changes: 80 additions & 11 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 @@ -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! {
Expand All @@ -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,
..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 +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,
},
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 = 445500;
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]
Expand Down

0 comments on commit 343c018

Please sign in to comment.