Skip to content

Commit

Permalink
test(blockifier): update test_insufficient_max_fee_reverts to include…
Browse files Browse the repository at this point in the history
… new resource bounds
  • Loading branch information
dorimedini-starkware committed Oct 10, 2024
1 parent b04682d commit a400602
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 20 deletions.
74 changes: 54 additions & 20 deletions crates/blockifier/src/transaction/account_transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1123,9 +1123,11 @@ fn test_max_fee_to_max_steps_conversion(
/// recorded and max_fee is charged.
fn test_insufficient_max_fee_reverts(
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)] cairo_version: CairoVersion,
) {
let gas_mode = resource_bounds.get_gas_vector_computation_mode();
let TestInitData { mut state, account_address, contract_address, mut nonce_manager } =
create_test_init_data(&block_context.chain_info, cairo_version);
let recursion_base_args = invoke_tx_args! {
Expand All @@ -1137,40 +1139,74 @@ fn test_insufficient_max_fee_reverts(
&mut state,
&block_context,
invoke_tx_args! {
resource_bounds: default_l1_resource_bounds,
resource_bounds,
nonce: nonce_manager.next(account_address),
calldata: recursive_function_calldata(&contract_address, 1, false),
..recursion_base_args.clone()
},
)
.unwrap();
assert!(!tx_execution_info1.is_reverted());
let actual_fee_depth1 = tx_execution_info1.receipt.fee;
let gas_price =
block_context.block_info.gas_prices.get_l1_gas_price_by_fee_type(&FeeType::Strk);
let gas_ammount = actual_fee_depth1.checked_div(gas_price).unwrap();

// Utility macro to verify the error and final fee is as expected, in each case of insufficient
// resource bounds.
macro_rules! verify_insufficient_bounds_result_by_gas_mode {
($execution_info:expr, $expected_error_in_l1_bounds_mode:expr) => {
assert!($execution_info.is_reverted());
// DA costs should be identical, regardless of bounds.
assert_eq!($execution_info.receipt.da_gas, tx_execution_info1.receipt.da_gas);
match gas_mode {
GasVectorComputationMode::NoL2Gas => {
assert!(
$execution_info
.revert_error
.unwrap()
.contains($expected_error_in_l1_bounds_mode)
);
// See usages of the macro for documentation on why the NoL2Gas case should
// assert equality.
assert_eq!($execution_info.receipt.fee, tx_execution_info1.receipt.fee);
}
GasVectorComputationMode::All => {
assert!($execution_info.revert_error.unwrap().contains("no remaining steps"));
assert!($execution_info.receipt.fee <= tx_execution_info1.receipt.fee);
}
};
};
}

// Invoke the `recurse` function with depth of 2 and the actual fee of depth 1 as max_fee.
// This call should fail due to insufficient max fee (steps bound based on max_fee is not so
// tight as to stop execution between iterations 1 and 2).
let resource_bounds_depth1 = match gas_mode {
GasVectorComputationMode::NoL2Gas => l1_resource_bounds(
tx_execution_info1.receipt.gas.l1_gas,
block_context.block_info.gas_prices.get_l1_gas_price_by_fee_type(&FeeType::Strk).into(),
),
GasVectorComputationMode::All => ValidResourceBounds::all_bounds_from_vectors(
&tx_execution_info1.receipt.gas,
block_context.block_info.gas_prices.get_gas_prices_by_fee_type(&FeeType::Strk),
),
};
let tx_execution_info2 = run_invoke_tx(
&mut state,
&block_context,
invoke_tx_args! {
resource_bounds: l1_resource_bounds(gas_ammount, gas_price.into()),
resource_bounds: resource_bounds_depth1,
nonce: nonce_manager.next(account_address),
calldata: recursive_function_calldata(&contract_address, 2, false),
..recursion_base_args.clone()
},
)
.unwrap();
assert!(tx_execution_info2.is_reverted());
assert!(tx_execution_info2.receipt.fee == actual_fee_depth1);
assert!(
tx_execution_info2
.revert_error
.unwrap()
.starts_with(&format!("Insufficient max {resource}", resource = Resource::L1Gas))
// In the deprecated bounds case, due to resource limit being estimated by steps, the execution
// will not fail due to insufficient resources; there are not enough steps in execution to hit
// the bound. Post-execution will fail due to insufficient max fee.
// The expected revert fee is therefore the same as the original fee (snap to bounds on
// post-execution error).
verify_insufficient_bounds_result_by_gas_mode!(
tx_execution_info2,
&format!("Insufficient max {resource}", resource = Resource::L1Gas)
);

// Invoke the `recurse` function with depth of 824 and the actual fee of depth 1 as max_fee.
Expand All @@ -1180,18 +1216,16 @@ fn test_insufficient_max_fee_reverts(
&mut state,
&block_context,
invoke_tx_args! {
resource_bounds: l1_resource_bounds(gas_ammount, gas_price.into()),
resource_bounds: resource_bounds_depth1,
nonce: nonce_manager.next(account_address),
calldata: recursive_function_calldata(&contract_address, 824, false),
..recursion_base_args
},
)
.unwrap();
assert!(tx_execution_info3.is_reverted());
assert!(tx_execution_info3.receipt.fee == actual_fee_depth1);
assert!(
tx_execution_info3.revert_error.unwrap().contains("RunResources has no remaining steps.")
);
// In the deprecated bounds case, the entire step limit is derived from 100% of all the
// bounds (only L1 gas), so expected revert fee is identical to the original fee.
verify_insufficient_bounds_result_by_gas_mode!(tx_execution_info3, "no remaining steps");
}

#[rstest]
Expand Down
21 changes: 21 additions & 0 deletions crates/starknet_api/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1139,6 +1139,27 @@ impl ValidResourceBounds {
pub fn create_for_testing() -> Self {
Self::L1Gas(ResourceBounds { max_amount: GasAmount(0), max_price_per_unit: GasPrice(1) })
}

/// Utility method to "zip" an amount vector and a price vector to get an AllResourceBounds.
#[cfg(any(feature = "testing", test))]
pub fn all_bounds_from_vectors(
gas: &crate::execution_resources::GasVector,
prices: &crate::block::GasPriceVector,
) -> Self {
let l1_gas = ResourceBounds {
max_amount: gas.l1_gas,
max_price_per_unit: prices.l1_gas_price.into(),
};
let l2_gas = ResourceBounds {
max_amount: gas.l2_gas,
max_price_per_unit: prices.l2_gas_price.into(),
};
let l1_data_gas = ResourceBounds {
max_amount: gas.l1_data_gas,
max_price_per_unit: prices.l1_data_gas_price.into(),
};
Self::AllResources(AllResourceBounds { l1_gas, l2_gas, l1_data_gas })
}
}

#[derive(
Expand Down

0 comments on commit a400602

Please sign in to comment.