From 608770a3ec2d47897f26c566d7764531e0ff5a4e Mon Sep 17 00:00:00 2001 From: Aviv Greenburg Date: Wed, 4 Sep 2024 14:50:32 +0300 Subject: [PATCH] refactor(blockifier): remove enforce fee from actual fee --- .../blockifier/src/concurrency/fee_utils.rs | 7 ++-- crates/blockifier/src/fee/actual_cost.rs | 13 ++++--- .../src/transaction/account_transaction.rs | 2 +- .../transaction/account_transactions_test.rs | 3 +- .../src/transaction/transactions_test.rs | 34 +++++++++++++------ 5 files changed, 36 insertions(+), 23 deletions(-) diff --git a/crates/blockifier/src/concurrency/fee_utils.rs b/crates/blockifier/src/concurrency/fee_utils.rs index e40f492319..8e8f13b694 100644 --- a/crates/blockifier/src/concurrency/fee_utils.rs +++ b/crates/blockifier/src/concurrency/fee_utils.rs @@ -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.") } } diff --git a/crates/blockifier/src/fee/actual_cost.rs b/crates/blockifier/src/fee/actual_cost.rs index c91f3dac4c..86f59e93a5 100644 --- a/crates/blockifier/src/fee/actual_cost.rs +++ b/crates/blockifier/src/fee/actual_cost.rs @@ -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; @@ -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); diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 31721656f5..caae61f9bc 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -359,7 +359,7 @@ impl AccountTransaction { charge_fee: bool, concurrency_mode: bool, ) -> TransactionExecutionResult> { - if !charge_fee || actual_fee == Fee(0) { + if !charge_fee { // Fee charging is not enforced in some transaction simulations and tests. return Ok(None); } diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index e846dc5062..94ad82a6e9 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -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. @@ -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, ) { @@ -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, diff --git a/crates/blockifier/src/transaction/transactions_test.rs b/crates/blockifier/src/transaction/transactions_test.rs index cfd055ba0f..efdd13a9d4 100644 --- a/crates/blockifier/src/transaction/transactions_test.rs +++ b/crates/blockifier/src/transaction/transactions_test.rs @@ -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, @@ -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( @@ -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(