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

feat(blockifier): update assert_actual_fee_in_bounds and add test #1118

Merged
merged 1 commit into from
Oct 1, 2024
Merged
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
14 changes: 5 additions & 9 deletions crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,15 +469,12 @@ impl AccountTransaction {
fn assert_actual_fee_in_bounds(tx_context: &Arc<TransactionContext>, actual_fee: Fee) {
match &tx_context.tx_info {
TransactionInfo::Current(context) => {
let ResourceBounds {
max_amount: max_l1_gas_amount,
max_price_per_unit: max_l1_gas_price,
} = context.l1_resource_bounds();
if actual_fee > Fee(u128::from(max_l1_gas_amount) * max_l1_gas_price) {
let max_fee = context.resource_bounds.max_possible_fee();
if actual_fee > max_fee {
panic!(
"Actual fee {:#?} exceeded bounds; max amount is {:#?}, max price is
{:#?}.",
actual_fee, max_l1_gas_amount, max_l1_gas_price
"Actual fee {:#?} exceeded bounds; max possible fee is {:#?} (computed \
from {:#?}).",
actual_fee, max_fee, context.resource_bounds
);
}
}
Expand All @@ -504,7 +501,6 @@ impl AccountTransaction {
return Ok(None);
}

// TODO(Amos, 8/04/2024): Add test for this assert.
Self::assert_actual_fee_in_bounds(&tx_context, actual_fee);

let fee_transfer_call_info = if concurrency_mode && !tx_context.is_sequencer_the_sender() {
Expand Down
53 changes: 53 additions & 0 deletions crates/blockifier/src/transaction/account_transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ use starknet_api::state::StorageKey;
use starknet_api::test_utils::invoke::InvokeTxArgs;
use starknet_api::test_utils::NonceManager;
use starknet_api::transaction::{
AllResourceBounds,
Calldata,
ContractAddressSalt,
DeclareTransactionV2,
Fee,
Resource,
ResourceBounds,
TransactionHash,
TransactionVersion,
ValidResourceBounds,
Expand Down Expand Up @@ -182,6 +184,57 @@ fn test_fee_enforcement(
assert_eq!(result.is_err(), enforce_fee);
}

#[rstest]
#[case::positive_case_deprecated_tx(true, true)]
#[case::positive_case_new_tx(true, false)]
#[should_panic(expected = "exceeded bounds; max fee is")]
#[case::negative_case_deprecated_tx(false, true)]
#[should_panic(expected = "exceeded bounds; max possible fee is")]
#[case::negative_case_new_tx(false, false)]
fn test_assert_actual_fee_in_bounds(
block_context: BlockContext,
#[case] positive_flow: bool,
#[case] deprecated_tx: bool,
) {
let actual_fee_offset = if positive_flow { 0 } else { 1 };
if deprecated_tx {
let max_fee = 100;
let tx = account_invoke_tx(invoke_tx_args! {
max_fee: Fee(max_fee),
version: TransactionVersion::ONE,
});
let context = Arc::new(block_context.to_tx_context(&tx));
AccountTransaction::assert_actual_fee_in_bounds(&context, Fee(max_fee + actual_fee_offset));
} else {
// All resources.
let l1_gas = ResourceBounds { max_amount: 2, max_price_per_unit: 3 };
let l2_gas = ResourceBounds { max_amount: 4, max_price_per_unit: 5 };
let l1_data_gas = ResourceBounds { max_amount: 6, max_price_per_unit: 7 };
let all_resource_bounds =
ValidResourceBounds::AllResources(AllResourceBounds { l1_gas, l2_gas, l1_data_gas });
let all_resource_fee = u128::from(l1_gas.max_amount) * l1_gas.max_price_per_unit
+ u128::from(l2_gas.max_amount) * l2_gas.max_price_per_unit
+ u128::from(l1_data_gas.max_amount) * l1_data_gas.max_price_per_unit
+ actual_fee_offset;

// L1 resources.
let l1_resource_bounds = ValidResourceBounds::L1Gas(l1_gas);
let l1_resource_fee =
u128::from(l1_gas.max_amount) * l1_gas.max_price_per_unit + actual_fee_offset;

for (bounds, actual_fee) in
[(all_resource_bounds, all_resource_fee), (l1_resource_bounds, l1_resource_fee)]
{
let tx = account_invoke_tx(invoke_tx_args! {
resource_bounds: bounds,
version: TransactionVersion::THREE,
});
let context = Arc::new(block_context.to_tx_context(&tx));
AccountTransaction::assert_actual_fee_in_bounds(&context, Fee(actual_fee));
}
}
}

#[rstest]
#[case(TransactionVersion::ZERO)]
#[case(TransactionVersion::ONE)]
Expand Down
Loading