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 17, 2024
1 parent 3df10e1 commit 41620d5
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 24 deletions.
14 changes: 9 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,15 @@ 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(); //aviv: new

// assert_eq!(
// //aviv: changes:
// // tx_execution_info.receipt.fee,
// // Fee(0),
// "Transaction with no fee transfer info must have zero fee."
// )
assert!(!charge_fee, "Transaction with no fee transfer info must have zero fee.")
}
}

Expand Down
17 changes: 13 additions & 4 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 @@ -99,11 +99,20 @@ impl TransactionReceipt {
)?;

// 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 {
// PREV:
// 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 {
// Fee(0)
// };
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/test_utils/prices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ fn fee_transfer_resources(
&mut EntryPointExecutionContext::new(
Arc::new(block_context.to_tx_context(&account_invoke_tx(InvokeTxArgs::default()))),
ExecutionMode::Execute,
false, // aviv: limit_steps_by_resources: can be enforce_fee() instead of false.?
false,
),
)
.unwrap()
Expand Down
3 changes: 2 additions & 1 deletion crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,8 @@ impl AccountTransaction {
charge_fee: bool,
concurrency_mode: bool,
) -> TransactionExecutionResult<Option<CallInfo>> {
if !charge_fee || actual_fee == Fee(0) {
// aviv: 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 @@ -205,7 +205,7 @@ 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));
// assert_eq!(tx_execution_info.receipt.fee, Fee(0)); // aviv: to remove?
}

// TODO(Dori, 15/9/2023): Convert version variance to attribute macro.
Expand Down Expand Up @@ -526,6 +526,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 @@ -544,6 +545,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
2 changes: 1 addition & 1 deletion crates/blockifier/src/transaction/objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl TransactionInfo {
TransactionInfo::Current(context) => {
let l1_bounds = context.l1_resource_bounds();
let max_amount: u128 = l1_bounds.max_amount.into();
max_amount * l1_bounds.max_price_per_unit > 0
max_amount * l1_bounds.max_price_per_unit > 0 //aviv: note : this is max fee
}
TransactionInfo::Deprecated(context) => context.max_fee != Fee(0),
}
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 @@ -1225,10 +1225,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 @@ -1247,7 +1248,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 @@ -1259,14 +1261,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 41620d5

Please sign in to comment.