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 Oct 13, 2024
1 parent 57822fd commit db4962d
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 14 deletions.
2 changes: 2 additions & 0 deletions crates/blockifier/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ pub const MAX_L1_GAS_AMOUNT: GasAmount = GasAmount(1000000);
pub const MAX_L1_GAS_PRICE: NonzeroGasPrice = DEFAULT_STRK_L1_GAS_PRICE;
pub const MAX_RESOURCE_COMMITMENT: Fee = MAX_L1_GAS_AMOUNT.nonzero_saturating_mul(MAX_L1_GAS_PRICE);
pub const MAX_FEE: Fee = MAX_L1_GAS_AMOUNT.nonzero_saturating_mul(DEFAULT_ETH_L1_GAS_PRICE);
pub const MAX_L2_GAS_PRICE: NonzeroGasPrice = DEFAULT_STRK_L2_GAS_PRICE;
pub const MAX_L1_DATA_GAS_PRICE: NonzeroGasPrice = DEFAULT_STRK_L1_DATA_GAS_PRICE;

// The amount of test-token allocated to the account in this test, set to a multiple of the max
// amount deprecated / non-deprecated transactions commit to paying.
Expand Down
103 changes: 89 additions & 14 deletions crates/blockifier/src/transaction/transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rstest::{fixture, rstest};
use starknet_api::block::GasPriceVector;
use starknet_api::contract_class::EntryPointType;
use starknet_api::core::{ChainId, ClassHash, ContractAddress, EthAddress, Nonce, PatriciaKey};
use starknet_api::execution_resources::GasVector;
use starknet_api::execution_resources::{GasAmount, GasVector};
use starknet_api::state::StorageKey;
use starknet_api::test_utils::invoke::InvokeTxArgs;
use starknet_api::test_utils::NonceManager;
Expand Down Expand Up @@ -104,8 +104,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 @@ -151,6 +153,8 @@ use crate::{
static VERSIONED_CONSTANTS: LazyLock<VersionedConstants> =
LazyLock::new(VersionedConstants::create_for_testing);

const ONE_GAS: GasAmount = GasAmount(1);

#[fixture]
fn tx_default_initial_gas() -> u64 {
VERSIONED_CONSTANTS.tx_default_initial_gas()
Expand Down Expand Up @@ -834,16 +838,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 @@ -885,10 +912,14 @@ fn test_estimate_minimal_gas_vector(

#[rstest]
fn test_max_fee_exceeds_balance(
block_context: BlockContext,
mut block_context: BlockContext,
max_l1_resource_bounds: ValidResourceBounds,
#[values(true, false)] use_kzg_da: bool,
#[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] account_cairo_version: CairoVersion,
) {
use starknet_api::execution_resources::GasAmount;

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 @@ -905,9 +936,9 @@ fn test_max_fee_exceeds_balance(

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(MAX_L1_GAS_PRICE).unwrap();
let invalid_resource_bounds =
l1_resource_bounds((balance_over_gas_price.0 + 1).into(), MAX_L1_GAS_PRICE.into());
let max_l1_gas_amount = BALANCE.checked_div(MAX_L1_GAS_PRICE).unwrap();
let invalid_l1_resource_bounds =
l1_resource_bounds((max_l1_gas_amount.0 + 1).into(), MAX_L1_GAS_PRICE.into());

// V1 Invoke.
let invalid_tx = account_invoke_tx(invoke_tx_args! {
Expand All @@ -919,8 +950,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 @@ -942,11 +973,55 @@ 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 = GasAmount(1652);
let min_l2_amount = GasAmount(445900);
let min_l1_data_amount = GasAmount(128);
let max_l2_gas_amount = BALANCE.checked_div(MAX_L2_GAS_PRICE).unwrap();
let max_l1_data_gas_amount = BALANCE.checked_div(MAX_L1_DATA_GAS_PRICE).unwrap();
// Ensure that in the following, the test won't fall on min amount too low.
assert!(max_l1_gas_amount / 3 + ONE_GAS >= min_l1_amount);
assert!(max_l2_gas_amount / 3 + ONE_GAS >= min_l2_amount);
assert!(max_l1_data_gas_amount / 3 + ONE_GAS >= min_l1_data_amount);
// 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_amount + ONE_GAS, min_l2_amount, min_l1_data_amount),
(min_l1_amount, max_l2_gas_amount + ONE_GAS, min_l1_data_amount),
(min_l1_amount, min_l2_amount, max_l1_data_gas_amount + ONE_GAS),
(
max_l1_gas_amount / 3 + ONE_GAS,
max_l2_gas_amount / 3 + ONE_GAS,
max_l1_data_gas_amount / 3 + ONE_GAS,
),
] {
let invalid_resource_bounds = ValidResourceBounds::AllResources(AllResourceBounds {
l1_gas: ResourceBounds {
max_amount: l1_max_amount,
max_price_per_unit: MAX_L1_GAS_PRICE.get(),
},
l2_gas: ResourceBounds {
max_amount: l2_max_amount,
max_price_per_unit: MAX_L2_GAS_PRICE.get(),
},
l1_data_gas: ResourceBounds {
max_amount: l1_data_max_amount,
max_price_per_unit: MAX_L1_DATA_GAS_PRICE.get(),
},
});
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
1 change: 1 addition & 0 deletions crates/starknet_api/src/execution_resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use crate::transaction::{Fee, Resource};
Deserialize,
Hash,
)]
#[cfg_attr(any(test, feature = "testing"), derive(derive_more::Div))]
pub struct GasAmount(pub u64);

impl From<GasAmount> for Felt {
Expand Down

0 comments on commit db4962d

Please sign in to comment.