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 29, 2024
1 parent 27784b2 commit 0b8953a
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 20 deletions.
83 changes: 63 additions & 20 deletions crates/blockifier/src/transaction/account_transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1161,9 +1161,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 @@ -1175,40 +1177,85 @@ 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_substring: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);
assert!($execution_info.revert_error.unwrap().contains($expected_error_substring));
match gas_mode {
GasVectorComputationMode::NoL2Gas => {
// In the L1 gas 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.
assert_eq!($execution_info.receipt.fee, tx_execution_info1.receipt.fee);
}
GasVectorComputationMode::All => {
// We do not expect the exact same fee in the All case.
// Unlike the L1 gas case, we have 3 separate bounds, and only L2 gas reached
// the limit. The other two bounds are not reached, so the fee should be less
// than the original fee.
// Also, the actual L2 gas amount is not the same as the original L2 gas amount
// of the successful tx, as the execution stopped before reaching the post
// execution receipt computation.
assert!($execution_info.receipt.fee > Fee(0));
assert!($execution_info.receipt.gas.l2_gas > GasAmount(0));
assert!($execution_info.receipt.fee <= tx_execution_info1.receipt.fee);
assert!(
$execution_info.receipt.gas.l2_gas <= tx_execution_info1.receipt.gas.l2_gas
);
}
};
};
}

// 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_used_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_used_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 L1 gas 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).
let overdraft_resource = match gas_mode {
GasVectorComputationMode::NoL2Gas => Resource::L1Gas,
GasVectorComputationMode::All => Resource::L2Gas,
};
verify_insufficient_bounds_result_by_gas_mode!(
tx_execution_info2,
&format!("Insufficient max {overdraft_resource}")
);

// Invoke the `recurse` function with depth of 824 and the actual fee of depth 1 as max_fee.
Expand All @@ -1218,18 +1265,14 @@ 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_used_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.")
);
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 @@ -1153,6 +1153,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 0b8953a

Please sign in to comment.