Skip to content

Commit

Permalink
feat(blockifier): update assert_actual_fee_in_bounds and add test
Browse files Browse the repository at this point in the history
Signed-off-by: Dori Medini <dori@starkware.co>
  • Loading branch information
dorimedini-starkware committed Sep 30, 2024
1 parent dc1730d commit b4c059b
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 9 deletions.
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
47 changes: 47 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,51 @@ 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]
#[case::negative_case_deprecated_tx(false, true)]
#[should_panic]
#[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 tx = account_invoke_tx(invoke_tx_args! {
max_fee: Fee(100),
version: TransactionVersion::ONE,
});
let context = Arc::new(block_context.to_tx_context(&tx));
AccountTransaction::assert_actual_fee_in_bounds(&context, Fee(100 + actual_fee_offset));
} else {
for (bounds, actual_fee) in [
(
ValidResourceBounds::AllResources(AllResourceBounds {
l1_gas: ResourceBounds { max_amount: 2, max_price_per_unit: 3 },
l2_gas: ResourceBounds { max_amount: 4, max_price_per_unit: 5 },
l1_data_gas: ResourceBounds { max_amount: 6, max_price_per_unit: 7 },
}),
2 * 3 + 4 * 5 + 6 * 7 + actual_fee_offset,
),
(
ValidResourceBounds::L1Gas(ResourceBounds { max_amount: 2, max_price_per_unit: 3 }),
2 * 3 + actual_fee_offset,
),
] {
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

0 comments on commit b4c059b

Please sign in to comment.