Skip to content

Commit

Permalink
feat(blockifier): update assert_actual_fee_in_bounds and add test (#1118
Browse files Browse the repository at this point in the history
)

Signed-off-by: Dori Medini <dori@starkware.co>
  • Loading branch information
dorimedini-starkware authored Oct 1, 2024
1 parent 47f4cce commit 0123ab2
Show file tree
Hide file tree
Showing 2 changed files with 58 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 @@ -457,15 +457,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 @@ -492,7 +489,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

0 comments on commit 0123ab2

Please sign in to comment.