Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

Commit

Permalink
Fail with an Err transactions whose calculated fee exceed max_fee (#…
Browse files Browse the repository at this point in the history
…892)

* Make tx fail when actual_fee exceeds max_fee

* Changed test

* Formatting

* Fix logic

* Leave fail only without charging

* Change test

* Fix test broken by better fee calc

* Fixed test fee

* Update fee on test_deploy_account

* Remove comment

---------

Co-authored-by: Juan Bono <juanbono94@gmail.com>
  • Loading branch information
2 people authored and fannyguthmann committed Aug 10, 2023
1 parent 3353b84 commit 42c1d95
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 13 deletions.
19 changes: 11 additions & 8 deletions src/transaction/fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,16 +154,18 @@ pub fn charge_fee<S: StateReader>(
block_context,
)?;

if actual_fee > max_fee {
// TODO: Charge max_fee
return Err(TransactionError::ActualFeeExceedsMaxFee(
actual_fee, max_fee,
));
}

let actual_fee = if tx_execution_context.version != 0.into()
&& tx_execution_context.version != *QUERY_VERSION_BASE
{
min(actual_fee, max_fee) * FEE_FACTOR
} else {
if actual_fee > max_fee {
return Err(TransactionError::ActualFeeExceedsMaxFee(
actual_fee, max_fee,
));
}
actual_fee
};

Expand All @@ -177,6 +179,7 @@ pub fn charge_fee<S: StateReader>(
actual_fee,
)?)
};

Ok((fee_transfer_info, actual_fee))
}

Expand Down Expand Up @@ -218,7 +221,7 @@ mod tests {
}

#[test]
fn test_charge_fee_v1_actual_fee_exceeds_max_fee_should_return_max_fee() {
fn test_charge_fee_v1_actual_fee_exceeds_max_fee_should_return_error() {
let mut state = CachedState::new(Arc::new(InMemoryStateReader::default()), None, None);
let mut tx_execution_context = TransactionExecutionContext {
version: 1.into(),
Expand All @@ -241,8 +244,8 @@ mod tests {
&mut tx_execution_context,
skip_fee_transfer,
)
.unwrap();
.unwrap_err();

assert_eq!(result.1, max_fee);
assert_matches!(result, TransactionError::ActualFeeExceedsMaxFee(_, _));
}
}
6 changes: 3 additions & 3 deletions src/transaction/invoke_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ mod tests {
}

#[test]
fn test_execute_invoke_actual_fee_exceeded_max_fee_should_charge_max_fee() {
fn test_execute_invoke_actual_fee_exceeded_max_fee_should_fail() {
let max_fee = 5;
let internal_invoke_function = InvokeFunction {
contract_address: Address(0.into()),
Expand Down Expand Up @@ -824,8 +824,8 @@ mod tests {

let tx = internal_invoke_function
.execute(&mut state, &block_context, 0)
.unwrap();
assert_eq!(tx.actual_fee, max_fee);
.unwrap_err();
assert_matches!(tx, TransactionError::ActualFeeExceedsMaxFee(_, _));
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions tests/internals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1459,7 +1459,7 @@ fn test_invoke_with_declarev2_tx() {
fn test_deploy_account() {
let (block_context, mut state) = create_account_tx_test_state().unwrap();

let expected_fee = 3684;
let expected_fee = 6157;

let deploy_account_tx = DeployAccount::new(
felt_to_hash(&TEST_ACCOUNT_CONTRACT_CLASS_HASH),
Expand Down Expand Up @@ -1576,7 +1576,7 @@ fn expected_deploy_account_states() -> (
CachedState<InMemoryStateReader>,
CachedState<InMemoryStateReader>,
) {
let fee = Felt252::from(3684);
let fee = Felt252::from(6157);
let mut state_before = CachedState::new(
Arc::new(InMemoryStateReader::new(
HashMap::from([
Expand Down

0 comments on commit 42c1d95

Please sign in to comment.