Skip to content

Commit

Permalink
refactor(blockifier): remove enforce fee from actual fee
Browse files Browse the repository at this point in the history
  • Loading branch information
avivg-starkware committed Sep 19, 2024
1 parent cf3b295 commit 608770a
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 23 deletions.
7 changes: 2 additions & 5 deletions crates/blockifier/src/concurrency/fee_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,8 @@ pub fn complete_fee_transfer_flow(
sequencer_balance,
);
} else {
assert_eq!(
tx_execution_info.receipt.fee,
Fee(0),
"Transaction with no fee transfer info must have zero fee."
)
let charge_fee = tx_context.tx_info.enforce_fee();
assert!(!charge_fee, "Transaction with no fee transfer info must have zero fee.")
}
}

Expand Down
13 changes: 8 additions & 5 deletions crates/blockifier/src/fee/actual_cost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::HashMap;

use cairo_vm::vm::runners::cairo_runner::ExecutionResources;
use starknet_api::core::ContractAddress;
use starknet_api::transaction::Fee;
use starknet_api::transaction::{Fee, TransactionVersion};

use crate::abi::constants as abi_constants;
use crate::context::TransactionContext;
Expand Down Expand Up @@ -98,12 +98,15 @@ impl TransactionReceipt {
&tx_context.get_gas_vector_computation_mode(),
)?;

// L1 handler transactions are not charged an L2 fee but it is compared to the L1 fee.
let fee = if tx_context.tx_info.enforce_fee() || tx_type == TransactionType::L1Handler {
tx_context.tx_info.get_fee_by_gas_vector(&tx_context.block_context.block_info, gas)
} else {
// To be backward compatible with the meaning of enforce fee in Declare V0.
let fee = if tx_type == TransactionType::Declare
&& tx_context.tx_info.signed_version() == TransactionVersion::ZERO
{
Fee(0)
} else {
tx_context.tx_info.get_fee_by_gas_vector(&tx_context.block_context.block_info, gas)
};

let da_gas = tx_resources
.starknet_resources
.get_state_changes_cost(tx_context.block_context.block_info.use_kzg_da);
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ impl AccountTransaction {
charge_fee: bool,
concurrency_mode: bool,
) -> TransactionExecutionResult<Option<CallInfo>> {
if !charge_fee || actual_fee == Fee(0) {
if !charge_fee {
// Fee charging is not enforced in some transaction simulations and tests.
return Ok(None);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ fn test_enforce_fee_false_works(block_context: BlockContext, #[case] version: Tr
)
.unwrap();
assert!(!tx_execution_info.is_reverted());
assert_eq!(tx_execution_info.receipt.fee, Fee(0));
}

// TODO(Dori, 15/9/2023): Convert version variance to attribute macro.
Expand Down Expand Up @@ -529,6 +528,7 @@ fn test_recursion_depth_exceeded(
fn test_revert_invoke(
block_context: BlockContext,
max_fee: Fee,
max_resource_bounds: ValidResourceBounds,
#[case] transaction_version: TransactionVersion,
#[case] fee_type: FeeType,
) {
Expand All @@ -547,6 +547,7 @@ fn test_revert_invoke(
&block_context,
invoke_tx_args! {
max_fee,
resource_bounds: max_resource_bounds,
sender_address: account_address,
calldata: create_calldata(
test_contract_address,
Expand Down
34 changes: 23 additions & 11 deletions crates/blockifier/src/transaction/transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1227,10 +1227,11 @@ fn test_declare_tx(
None,
std::iter::empty(),
);

// For V0 transactions, the max fee is always 0.
let max_fee = if tx_version == TransactionVersion::ZERO { Fee(0) } else { Fee(MAX_FEE) };
let account_tx = declare_tx(
declare_tx_args! {
max_fee: Fee(MAX_FEE),
max_fee,
sender_address,
version: tx_version,
resource_bounds: max_resource_bounds,
Expand All @@ -1249,7 +1250,8 @@ fn test_declare_tx(
);
let fee_type = &account_tx.fee_type();
let tx_context = &block_context.to_tx_context(&account_tx);
let actual_execution_info = account_tx.execute(state, block_context, true, true).unwrap();
let actual_execution_info =
account_tx.execute(state, block_context, tx_context.tx_info.enforce_fee(), true).unwrap(); //aviv: changed from true, true

// Build expected validate call info.
let expected_validate_call_info = declare_validate_callinfo(
Expand All @@ -1261,14 +1263,24 @@ fn test_declare_tx(
);

// Build expected fee transfer call info.
let expected_actual_fee =
actual_execution_info.receipt.resources.calculate_tx_fee(block_context, fee_type).unwrap();
let expected_fee_transfer_call_info = expected_fee_transfer_call_info(
tx_context,
sender_address,
expected_actual_fee,
FeatureContract::ERC20(CairoVersion::Cairo0).get_class_hash(),
);
// aviv added 'if' condition. not pretty solution:
let expected_actual_fee = if tx_context.tx_info.signed_version() == TransactionVersion::ZERO {
Fee(0)
} else {
actual_execution_info.receipt.resources.calculate_tx_fee(block_context, fee_type).unwrap()
};
// aviv added 'if' condition. not pretty solution:
let expected_fee_transfer_call_info =
if tx_context.tx_info.signed_version() == TransactionVersion::ZERO {
None
} else {
expected_fee_transfer_call_info(
tx_context,
sender_address,
expected_actual_fee,
FeatureContract::ERC20(CairoVersion::Cairo0).get_class_hash(),
)
};

let da_gas = starknet_resources.get_state_changes_cost(use_kzg_da);
let expected_cairo_resources = get_expected_cairo_resources(
Expand Down

0 comments on commit 608770a

Please sign in to comment.