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

Commit

Permalink
refactor: make transactiontreceipt substuct of transactionexecutionin…
Browse files Browse the repository at this point in the history
…fo (#1972)
  • Loading branch information
AvivYossef-starkware authored Jun 9, 2024
1 parent 605b4bd commit 99527a3
Show file tree
Hide file tree
Showing 13 changed files with 148 additions and 106 deletions.
2 changes: 1 addition & 1 deletion crates/blockifier/src/blockifier/transaction_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ impl<S: StateReader> TransactionExecutor<S> {
&transactional_state,
&tx_state_changes_keys,
&tx_execution_info.summarize(),
&tx_execution_info.actual_resources,
&tx_execution_info.transaction_receipt.resources,
)?;
transactional_state.commit();
Ok(tx_execution_info)
Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/concurrency/worker_logic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> {
&tx_versioned_state,
&tx_state_changes_keys,
&tx_execution_info.summarize(),
&tx_execution_info.actual_resources,
&tx_execution_info.transaction_receipt.resources,
);
if let Err(error) = bouncer_result {
match error {
Expand Down Expand Up @@ -254,7 +254,7 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> {
add_fee_to_sequencer_balance(
tx_context.fee_token_address(),
&mut tx_versioned_state,
tx_execution_info.actual_fee,
tx_execution_info.transaction_receipt.fee,
self.block_context,
sequencer_balance_value_low,
sequencer_balance_value_high,
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/concurrency/worker_logic_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ fn test_worker_execute() {
let execution_output = worker_executor.execution_outputs[tx_index].lock().unwrap();
let execution_output = execution_output.as_ref().unwrap();
let result = execution_output.result.as_ref().unwrap();
let account_balance = BALANCE - result.actual_fee.0;
let account_balance = BALANCE - result.transaction_receipt.fee.0;
assert!(!result.is_reverted());

let erc20 = FeatureContract::ERC20;
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/fee/actual_cost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ struct TransactionReceiptParameters<'a, T: Iterator<Item = &'a CallInfo> + Clone

// TODO(Gilad): Use everywhere instead of passing the `actual_{fee,resources}` tuple, which often
// get passed around together.
#[derive(Default)]
#[derive(Default, Debug, PartialEq)]
pub struct TransactionReceipt {
pub fee: Fee,
pub gas: GasVector,
Expand Down
6 changes: 4 additions & 2 deletions crates/blockifier/src/fee/actual_cost_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,8 @@ fn test_calculate_tx_gas_usage(#[values(false, true)] use_kzg_da: bool) {
assert_eq!(
starknet_resources.to_gas_vector(versioned_constants, use_kzg_da),
tx_execution_info
.actual_resources
.transaction_receipt
.resources
.starknet_resources
.to_gas_vector(versioned_constants, use_kzg_da)
);
Expand Down Expand Up @@ -371,7 +372,8 @@ fn test_calculate_tx_gas_usage(#[values(false, true)] use_kzg_da: bool) {
assert_eq!(
starknet_resources.to_gas_vector(versioned_constants, use_kzg_da),
tx_execution_info
.actual_resources
.transaction_receipt
.resources
.starknet_resources
.to_gas_vector(versioned_constants, use_kzg_da)
);
Expand Down
10 changes: 6 additions & 4 deletions crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -674,11 +674,13 @@ impl<U: UpdatableState> ExecutableTransaction<U> for AccountTransaction {
validate_call_info,
execute_call_info,
fee_transfer_call_info,
actual_fee: final_fee,
da_gas: final_da_gas,
actual_resources: final_resources,
transaction_receipt: TransactionReceipt {
fee: final_fee,
da_gas: final_da_gas,
resources: final_resources,
gas: total_gas,
},
revert_error,
total_gas,
};
Ok(tx_execution_info)
}
Expand Down
69 changes: 37 additions & 32 deletions crates/blockifier/src/transaction/account_transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,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.actual_fee, Fee(0));
assert_eq!(tx_execution_info.transaction_receipt.fee, Fee(0));
}

// TODO(Dori, 15/9/2023): Convert version variance to attribute macro.
Expand Down Expand Up @@ -457,12 +457,12 @@ fn test_revert_invoke(
state
.get_fee_token_balance(account_address, chain_info.fee_token_address(&fee_type))
.unwrap(),
(stark_felt!(BALANCE - tx_execution_info.actual_fee.0), stark_felt!(0_u8))
(stark_felt!(BALANCE - tx_execution_info.transaction_receipt.fee.0), stark_felt!(0_u8))
);
assert_eq!(state.get_nonce_at(account_address).unwrap(), nonce_manager.next(account_address));

// Check that reverted steps are taken into account.
assert!(tx_execution_info.actual_resources.n_reverted_steps > 0);
assert!(tx_execution_info.transaction_receipt.resources.n_reverted_steps > 0);

// Check that execution state changes were reverted.
assert_eq!(
Expand Down Expand Up @@ -616,8 +616,8 @@ fn test_reverted_reach_steps_limit(
},
)
.unwrap();
let n_steps_0 = result.actual_resources.total_charged_steps();
let actual_fee_0 = result.actual_fee.0;
let n_steps_0 = result.transaction_receipt.resources.total_charged_steps();
let actual_fee_0 = result.transaction_receipt.fee.0;
// Ensure the transaction was not reverted.
assert!(!result.is_reverted());

Expand All @@ -632,8 +632,8 @@ fn test_reverted_reach_steps_limit(
},
)
.unwrap();
let n_steps_1 = result.actual_resources.total_charged_steps();
let actual_fee_1 = result.actual_fee.0;
let n_steps_1 = result.transaction_receipt.resources.total_charged_steps();
let actual_fee_1 = result.transaction_receipt.fee.0;
// Ensure the transaction was not reverted.
assert!(!result.is_reverted());

Expand All @@ -659,8 +659,8 @@ fn test_reverted_reach_steps_limit(
},
)
.unwrap();
let n_steps_fail = result.actual_resources.total_charged_steps();
let actual_fee_fail: u128 = result.actual_fee.0;
let n_steps_fail = result.transaction_receipt.resources.total_charged_steps();
let actual_fee_fail: u128 = result.transaction_receipt.fee.0;
// Ensure the transaction was reverted.
assert!(result.is_reverted());

Expand All @@ -680,8 +680,8 @@ fn test_reverted_reach_steps_limit(
},
)
.unwrap();
let n_steps_fail_next = result.actual_resources.total_charged_steps();
let actual_fee_fail_next: u128 = result.actual_fee.0;
let n_steps_fail_next = result.transaction_receipt.resources.total_charged_steps();
let actual_fee_fail_next: u128 = result.transaction_receipt.fee.0;
// Ensure the transaction was reverted.
assert!(result.is_reverted());

Expand Down Expand Up @@ -720,9 +720,9 @@ fn test_n_reverted_steps(
.unwrap();
// Ensure the transaction was reverted.
assert!(result.is_reverted());
let mut actual_resources_0 = result.actual_resources.clone();
let n_steps_0 = result.actual_resources.total_charged_steps();
let actual_fee_0 = result.actual_fee.0;
let mut actual_resources_0 = result.transaction_receipt.resources.clone();
let n_steps_0 = result.transaction_receipt.resources.total_charged_steps();
let actual_fee_0 = result.transaction_receipt.fee.0;

// Invoke the `recursive_fail` function with 1 iterations. This call should fail.
let result = run_invoke_tx(
Expand All @@ -737,9 +737,9 @@ fn test_n_reverted_steps(
.unwrap();
// Ensure the transaction was reverted.
assert!(result.is_reverted());
let actual_resources_1 = result.actual_resources;
let actual_resources_1 = result.transaction_receipt.resources;
let n_steps_1 = actual_resources_1.total_charged_steps();
let actual_fee_1 = result.actual_fee.0;
let actual_fee_1 = result.transaction_receipt.fee.0;

// Invoke the `recursive_fail` function with 2 iterations. This call should fail.
let result = run_invoke_tx(
Expand All @@ -752,8 +752,8 @@ fn test_n_reverted_steps(
},
)
.unwrap();
let n_steps_2 = result.actual_resources.total_charged_steps();
let actual_fee_2 = result.actual_fee.0;
let n_steps_2 = result.transaction_receipt.resources.total_charged_steps();
let actual_fee_2 = result.transaction_receipt.fee.0;
// Ensure the transaction was reverted.
assert!(result.is_reverted());

Expand Down Expand Up @@ -784,8 +784,8 @@ fn test_n_reverted_steps(
},
)
.unwrap();
let n_steps_100 = result.actual_resources.total_charged_steps();
let actual_fee_100 = result.actual_fee.0;
let n_steps_100 = result.transaction_receipt.resources.total_charged_steps();
let actual_fee_100 = result.transaction_receipt.fee.0;
// Ensure the transaction was reverted.
assert!(result.is_reverted());

Expand Down Expand Up @@ -832,9 +832,10 @@ fn test_max_fee_to_max_steps_conversion(
let execution_context1 = EntryPointExecutionContext::new_invoke(tx_context1, true).unwrap();
let max_steps_limit1 = execution_context1.vm_run_resources.get_n_steps();
let tx_execution_info1 = account_tx1.execute(&mut state, &block_context, true, true).unwrap();
let n_steps1 = tx_execution_info1.actual_resources.vm_resources.n_steps;
let n_steps1 = tx_execution_info1.transaction_receipt.resources.vm_resources.n_steps;
let gas_used_vector1 = tx_execution_info1
.actual_resources
.transaction_receipt
.resources
.to_gas_vector(&block_context.versioned_constants, block_context.block_info.use_kzg_da)
.unwrap();

Expand All @@ -851,22 +852,26 @@ fn test_max_fee_to_max_steps_conversion(
let execution_context2 = EntryPointExecutionContext::new_invoke(tx_context2, true).unwrap();
let max_steps_limit2 = execution_context2.vm_run_resources.get_n_steps();
let tx_execution_info2 = account_tx2.execute(&mut state, &block_context, true, true).unwrap();
let n_steps2 = tx_execution_info2.actual_resources.vm_resources.n_steps;
let n_steps2 = tx_execution_info2.transaction_receipt.resources.vm_resources.n_steps;
let gas_used_vector2 = tx_execution_info2
.actual_resources
.transaction_receipt
.resources
.to_gas_vector(&block_context.versioned_constants, block_context.block_info.use_kzg_da)
.unwrap();

// Test that steps limit doubles as max_fee doubles, but actual consumed steps and fee remains.
assert_eq!(max_steps_limit2.unwrap(), 2 * max_steps_limit1.unwrap());
assert_eq!(tx_execution_info1.actual_fee.0, tx_execution_info2.actual_fee.0);
assert_eq!(
tx_execution_info1.transaction_receipt.fee.0,
tx_execution_info2.transaction_receipt.fee.0
);
// TODO(Ori, 1/2/2024): Write an indicative expect message explaining why the conversion works.
// TODO(Aner, 21/01/24): verify test compliant with 4844 (or modify accordingly).
assert_eq!(
actual_gas_used,
u64::try_from(gas_used_vector2.l1_gas).expect("Failed to convert u128 to u64.")
);
assert_eq!(actual_fee, tx_execution_info2.actual_fee.0);
assert_eq!(actual_fee, tx_execution_info2.transaction_receipt.fee.0);
assert_eq!(n_steps1, n_steps2);
assert_eq!(gas_used_vector1, gas_used_vector2);
}
Expand Down Expand Up @@ -898,7 +903,7 @@ fn test_insufficient_max_fee_reverts(
)
.unwrap();
assert!(!tx_execution_info1.is_reverted());
let actual_fee_depth1 = tx_execution_info1.actual_fee;
let actual_fee_depth1 = tx_execution_info1.transaction_receipt.fee;

// Invoke the `recurse` function with depth of 2 and the actual fee of depth 1 as max_fee.
// This call should fail due to insufficient max fee (steps bound based on max_fee is not so
Expand All @@ -915,7 +920,7 @@ fn test_insufficient_max_fee_reverts(
)
.unwrap();
assert!(tx_execution_info2.is_reverted());
assert!(tx_execution_info2.actual_fee == actual_fee_depth1);
assert!(tx_execution_info2.transaction_receipt.fee == actual_fee_depth1);
assert!(tx_execution_info2.revert_error.unwrap().starts_with("Insufficient max fee"));

// Invoke the `recurse` function with depth of 824 and the actual fee of depth 1 as max_fee.
Expand All @@ -933,7 +938,7 @@ fn test_insufficient_max_fee_reverts(
)
.unwrap();
assert!(tx_execution_info3.is_reverted());
assert!(tx_execution_info3.actual_fee == actual_fee_depth1);
assert!(tx_execution_info3.transaction_receipt.fee == actual_fee_depth1);
assert!(
tx_execution_info3.revert_error.unwrap().contains("RunResources has no remaining steps.")
);
Expand Down Expand Up @@ -1037,7 +1042,7 @@ fn test_count_actual_storage_changes(
let account_tx = account_invoke_tx(invoke_args.clone());
let execution_info = account_tx.execute_raw(&mut state, &block_context, true, true).unwrap();

let fee_1 = execution_info.actual_fee;
let fee_1 = execution_info.transaction_receipt.fee;
let state_changes_1 = state.get_actual_state_changes().unwrap();

let cell_write_storage_change = ((contract_address, storage_key!(15_u8)), stark_felt!(1_u8));
Expand Down Expand Up @@ -1081,7 +1086,7 @@ fn test_count_actual_storage_changes(
});
let execution_info = account_tx.execute_raw(&mut state, &block_context, true, true).unwrap();

let fee_2 = execution_info.actual_fee;
let fee_2 = execution_info.transaction_receipt.fee;
let state_changes_2 = state.get_actual_state_changes().unwrap();

expected_sequencer_total_fee += Felt252::from(fee_2.0);
Expand Down Expand Up @@ -1118,7 +1123,7 @@ fn test_count_actual_storage_changes(
});
let execution_info = account_tx.execute_raw(&mut state, &block_context, true, true).unwrap();

let fee_transfer = execution_info.actual_fee;
let fee_transfer = execution_info.transaction_receipt.fee;
let state_changes_transfer = state.get_actual_state_changes().unwrap();
let transfer_receipient_storage_change = (
(fee_token_address, get_fee_token_var_address(contract_address!(recipient))),
Expand Down
21 changes: 13 additions & 8 deletions crates/blockifier/src/transaction/execution_flavors_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,18 +108,23 @@ fn check_gas_and_fee(
) {
assert_eq!(
tx_execution_info
.actual_resources
.transaction_receipt
.resources
.to_gas_vector(&block_context.versioned_constants, block_context.block_info.use_kzg_da)
.unwrap()
.l1_gas,
expected_actual_gas.into()
);

assert_eq!(tx_execution_info.actual_fee, expected_actual_fee);
assert_eq!(tx_execution_info.transaction_receipt.fee, expected_actual_fee);
// Future compatibility: resources other than the L1 gas usage may affect the fee (currently,
// `calculate_tx_fee` is simply the result of `calculate_tx_gas_usage_vector` times gas price).
assert_eq!(
tx_execution_info.actual_resources.calculate_tx_fee(block_context, fee_type).unwrap(),
tx_execution_info
.transaction_receipt
.resources
.calculate_tx_fee(block_context, fee_type)
.unwrap(),
expected_cost_of_resources
);
}
Expand Down Expand Up @@ -485,8 +490,8 @@ fn test_simulate_validate_charge_fee_mid_execution(
// availability), hence the actual resources may exceed the senders bounds after all.
if charge_fee { limited_gas_used } else { unlimited_gas_used },
if charge_fee { fee_bound } else { unlimited_fee },
// Complete resources used are reported as actual_resources; but only the charged final fee
// is shown in actual_fee.
// Complete resources used are reported as transaction_receipt.resources; but only the
// charged final fee is shown in actual_fee.
if charge_fee { limited_fee } else { unlimited_fee },
);
let current_balance = check_balance(
Expand Down Expand Up @@ -524,9 +529,9 @@ fn test_simulate_validate_charge_fee_mid_execution(
.execute(&mut state, &low_step_block_context, charge_fee, validate)
.unwrap();
assert!(tx_execution_info.revert_error.clone().unwrap().contains("no remaining steps"));
// Complete resources used are reported as actual_resources; but only the charged final fee is
// shown in actual_fee. As a sanity check, verify that the fee derived directly from the
// consumed resources is also equal to the expected fee.
// Complete resources used are reported as transaction_receipt.resources; but only the charged
// final fee is shown in actual_fee. As a sanity check, verify that the fee derived directly
// from the consumed resources is also equal to the expected fee.
check_gas_and_fee(
&block_context,
&tx_execution_info,
Expand Down
20 changes: 8 additions & 12 deletions crates/blockifier/src/transaction/objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::abi::constants as abi_constants;
use crate::blockifier::block::BlockInfo;
use crate::execution::call_info::{CallInfo, ExecutionSummary, MessageL1CostInfo, OrderedEvent};
use crate::execution::execution_utils::{felt_to_stark_felt, stark_felt_to_felt};
use crate::fee::actual_cost::TransactionReceipt;
use crate::fee::eth_gas_constants;
use crate::fee::fee_utils::{calculate_l1_gas_by_vm_usage, get_fee_by_gas_vector};
use crate::fee::gas_usage::{
Expand Down Expand Up @@ -203,19 +204,14 @@ pub struct TransactionExecutionInfo {
pub execute_call_info: Option<CallInfo>,
/// Fee transfer call info; [None] for `L1Handler`.
pub fee_transfer_call_info: Option<CallInfo>,
/// The actual fee that was charged (in Wei).
pub actual_fee: Fee,
/// Actual gas consumption the transaction is charged for data availability.
pub da_gas: GasVector,
/// Actual execution resources the transaction is charged for,
/// including L1 gas and additional OS resources estimation.
pub actual_resources: TransactionResources,
/// Error string for reverted transactions; [None] if transaction execution was successful.
// TODO(Dori, 1/8/2023): If the `Eq` and `PartialEq` traits are removed, or implemented on all
// internal structs in this enum, this field should be `Option<TransactionExecutionError>`.
pub revert_error: Option<String>,

pub total_gas: GasVector,
/// The receipt of the transaction.
/// Including the actual fee that was charged (in units of the relevant fee token),
/// actual gas consumption the transaction is charged for data availability,
/// actual execution resources the transaction is charged for
/// (including L1 gas and additional OS resources estimation),
/// and total gas consumed.
pub transaction_receipt: TransactionReceipt,
}

impl TransactionExecutionInfo {
Expand Down
Loading

0 comments on commit 99527a3

Please sign in to comment.