Skip to content

Commit

Permalink
refactor(blockifier): rename transaction_receipt to receipt (#336)
Browse files Browse the repository at this point in the history
  • Loading branch information
Yoni-Starkware authored Aug 6, 2024
1 parent bfc88ee commit 92d201d
Show file tree
Hide file tree
Showing 15 changed files with 103 additions and 135 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 @@ -103,7 +103,7 @@ impl<S: StateReader> TransactionExecutor<S> {
&transactional_state,
&tx_state_changes_keys,
&tx_execution_info.summarize(),
&tx_execution_info.transaction_receipt.resources,
&tx_execution_info.receipt.resources,
)?;
transactional_state.commit();
Ok(tx_execution_info)
Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/concurrency/fee_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,13 @@ pub fn complete_fee_transfer_flow(
add_fee_to_sequencer_balance(
tx_context.fee_token_address(),
state,
tx_execution_info.transaction_receipt.fee,
tx_execution_info.receipt.fee,
&tx_context.block_context,
sequencer_balance,
);
} else {
assert_eq!(
tx_execution_info.transaction_receipt.fee,
tx_execution_info.receipt.fee,
Fee(0),
"Transaction with no fee transfer info must have zero fee."
)
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/concurrency/worker_logic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> {
&tx_versioned_state,
&tx_state_changes_keys,
&tx_execution_info.summarize(),
&tx_execution_info.transaction_receipt.resources,
&tx_execution_info.receipt.resources,
);
if let Err(error) = bouncer_result {
match error {
Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/concurrency/worker_logic_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ pub fn test_commit_tx() {
let actual_fee = if should_fail_execution {
0
} else {
execution_result.as_ref().unwrap().transaction_receipt.fee.0
execution_result.as_ref().unwrap().receipt.fee.0
};
if !should_fail_execution {
assert!(!execution_result.as_ref().unwrap().is_reverted());
Expand Down Expand Up @@ -344,7 +344,7 @@ fn test_worker_execute(max_resource_bounds: ResourceBoundsMapping) {
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.transaction_receipt.fee.0;
let account_balance = BALANCE - result.receipt.fee.0;
assert!(!result.is_reverted());

let erc20 = FeatureContract::ERC20(CairoVersion::Cairo0);
Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/fee/actual_cost_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ fn test_calculate_tx_gas_usage(
assert_eq!(
starknet_resources.to_gas_vector(versioned_constants, use_kzg_da),
tx_execution_info
.transaction_receipt
.receipt
.resources
.starknet_resources
.to_gas_vector(versioned_constants, use_kzg_da)
Expand Down Expand Up @@ -378,7 +378,7 @@ fn test_calculate_tx_gas_usage(
assert_eq!(
starknet_resources.to_gas_vector(versioned_constants, use_kzg_da),
tx_execution_info
.transaction_receipt
.receipt
.resources
.starknet_resources
.to_gas_vector(versioned_constants, use_kzg_da)
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/state/cached_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ impl StateChangesKeys {
tx_result: &TransactionExecutionInfo,
concurrency_mode: bool,
) {
let actual_fee = tx_result.transaction_receipt.fee.0;
let actual_fee = tx_result.receipt.fee.0;
let sequencer_address = tx_context.block_context.block_info.sequencer_address;
if concurrency_mode && !tx_context.is_sequencer_the_sender() && actual_fee > 0 {
// Add the deleted sequencer balance key to the storage keys.
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 @@ -691,7 +691,7 @@ impl<U: UpdatableState> ExecutableTransaction<U> for AccountTransaction {
validate_call_info,
execute_call_info,
fee_transfer_call_info,
transaction_receipt: TransactionReceipt {
receipt: TransactionReceipt {
fee: final_fee,
da_gas: final_da_gas,
resources: final_resources,
Expand Down
73 changes: 35 additions & 38 deletions crates/blockifier/src/transaction/account_transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ fn test_circuit(block_context: BlockContext, max_resource_bounds: ResourceBounds
.unwrap();

assert!(tx_execution_info.revert_error.is_none());
assert_eq!(tx_execution_info.transaction_receipt.gas, GasVector::from_l1_gas(6682));
assert_eq!(tx_execution_info.receipt.gas, GasVector::from_l1_gas(6682));
}

#[rstest]
Expand Down Expand Up @@ -145,11 +145,11 @@ fn test_rc96_holes(block_context: BlockContext, max_resource_bounds: ResourceBou

assert!(!tx_execution_info.is_reverted());
assert_eq!(
tx_execution_info.transaction_receipt.resources.vm_resources.builtin_instance_counter
tx_execution_info.receipt.resources.vm_resources.builtin_instance_counter
[&BuiltinName::range_check96],
24
);
assert_eq!(tx_execution_info.transaction_receipt.gas, GasVector::from_l1_gas(6598));
assert_eq!(tx_execution_info.receipt.gas, GasVector::from_l1_gas(6598));
}

#[rstest]
Expand Down Expand Up @@ -197,7 +197,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.transaction_receipt.fee, Fee(0));
assert_eq!(tx_execution_info.receipt.fee, Fee(0));
}

// TODO(Dori, 15/9/2023): Convert version variance to attribute macro.
Expand Down Expand Up @@ -559,12 +559,12 @@ fn test_revert_invoke(
state
.get_fee_token_balance(account_address, chain_info.fee_token_address(&fee_type))
.unwrap(),
(felt!(BALANCE - tx_execution_info.transaction_receipt.fee.0), felt!(0_u8))
(felt!(BALANCE - tx_execution_info.receipt.fee.0), 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.transaction_receipt.resources.n_reverted_steps > 0);
assert!(tx_execution_info.receipt.resources.n_reverted_steps > 0);

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

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

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

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

Expand Down Expand Up @@ -821,9 +821,9 @@ fn test_n_reverted_steps(
.unwrap();
// Ensure the transaction was reverted.
assert!(result.is_reverted());
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;
let mut actual_resources_0 = result.receipt.resources.clone();
let n_steps_0 = result.receipt.resources.total_charged_steps();
let actual_fee_0 = result.receipt.fee.0;

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

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

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

Expand Down Expand Up @@ -933,9 +933,9 @@ 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.transaction_receipt.resources.vm_resources.n_steps;
let n_steps1 = tx_execution_info1.receipt.resources.vm_resources.n_steps;
let gas_used_vector1 = tx_execution_info1
.transaction_receipt
.receipt
.resources
.to_gas_vector(&block_context.versioned_constants, block_context.block_info.use_kzg_da)
.unwrap();
Expand All @@ -953,26 +953,23 @@ 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.transaction_receipt.resources.vm_resources.n_steps;
let n_steps2 = tx_execution_info2.receipt.resources.vm_resources.n_steps;
let gas_used_vector2 = tx_execution_info2
.transaction_receipt
.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.transaction_receipt.fee.0,
tx_execution_info2.transaction_receipt.fee.0
);
assert_eq!(tx_execution_info1.receipt.fee.0, tx_execution_info2.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.transaction_receipt.fee.0);
assert_eq!(actual_fee, tx_execution_info2.receipt.fee.0);
assert_eq!(n_steps1, n_steps2);
assert_eq!(gas_used_vector1, gas_used_vector2);
}
Expand Down Expand Up @@ -1004,7 +1001,7 @@ fn test_insufficient_max_fee_reverts(
)
.unwrap();
assert!(!tx_execution_info1.is_reverted());
let actual_fee_depth1 = tx_execution_info1.transaction_receipt.fee;
let actual_fee_depth1 = tx_execution_info1.receipt.fee;
let gas_price = u128::from(block_context.block_info.gas_prices.strk_l1_gas_price);
let gas_ammount = u64::try_from(actual_fee_depth1.0 / gas_price).unwrap();

Expand All @@ -1023,7 +1020,7 @@ fn test_insufficient_max_fee_reverts(
)
.unwrap();
assert!(tx_execution_info2.is_reverted());
assert!(tx_execution_info2.transaction_receipt.fee == actual_fee_depth1);
assert!(tx_execution_info2.receipt.fee == actual_fee_depth1);
assert!(tx_execution_info2.revert_error.unwrap().starts_with("Insufficient max L1 gas:"));

// Invoke the `recurse` function with depth of 824 and the actual fee of depth 1 as max_fee.
Expand All @@ -1041,7 +1038,7 @@ fn test_insufficient_max_fee_reverts(
)
.unwrap();
assert!(tx_execution_info3.is_reverted());
assert!(tx_execution_info3.transaction_receipt.fee == actual_fee_depth1);
assert!(tx_execution_info3.receipt.fee == actual_fee_depth1);
assert!(
tx_execution_info3.revert_error.unwrap().contains("RunResources has no remaining steps.")
);
Expand Down Expand Up @@ -1148,7 +1145,7 @@ fn test_count_actual_storage_changes(
let execution_info =
account_tx.execute_raw(&mut state, &block_context, execution_flags).unwrap();

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

let cell_write_storage_change = ((contract_address, storage_key!(15_u8)), felt!(1_u8));
Expand Down Expand Up @@ -1191,7 +1188,7 @@ fn test_count_actual_storage_changes(
let execution_info =
account_tx.execute_raw(&mut state, &block_context, execution_flags).unwrap();

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

expected_sequencer_total_fee += Felt::from(fee_2.0);
Expand Down Expand Up @@ -1229,7 +1226,7 @@ fn test_count_actual_storage_changes(
let execution_info =
account_tx.execute_raw(&mut state, &block_context, execution_flags).unwrap();

let fee_transfer = execution_info.transaction_receipt.fee;
let fee_transfer = execution_info.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
14 changes: 5 additions & 9 deletions crates/blockifier/src/transaction/execution_flavors_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,23 +127,19 @@ fn check_gas_and_fee(
) {
assert_eq!(
tx_execution_info
.transaction_receipt
.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.transaction_receipt.fee, expected_actual_fee);
assert_eq!(tx_execution_info.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
.transaction_receipt
.resources
.calculate_tx_fee(block_context, fee_type)
.unwrap(),
tx_execution_info.receipt.resources.calculate_tx_fee(block_context, fee_type).unwrap(),
expected_cost_of_resources
);
}
Expand Down Expand Up @@ -514,7 +510,7 @@ 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 transaction_receipt.resources; but only the
// Complete resources used are reported as receipt.resources; but only the
// charged final fee is shown in actual_fee.
if charge_fee { limited_fee } else { unlimited_fee },
);
Expand Down Expand Up @@ -547,7 +543,7 @@ 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 transaction_receipt.resources; but only the charged
// Complete resources used are reported as 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(
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 @@ -222,7 +222,7 @@ pub struct TransactionExecutionInfo {
/// actual execution resources the transaction is charged for
/// (including L1 gas and additional OS resources estimation),
/// and total gas consumed.
pub transaction_receipt: TransactionReceipt,
pub receipt: TransactionReceipt,
}

impl TransactionExecutionInfo {
Expand Down
Loading

0 comments on commit 92d201d

Please sign in to comment.