Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(fee): tx_resources depend on resource bounds signature #455

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions crates/blockifier/src/fee/actual_cost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,7 @@ impl TransactionReceipt {
vm_resources: cairo_resources,
n_reverted_steps: reverted_steps,
};

let gas = tx_resources.to_gas_vector(
&tx_context.block_context.versioned_constants,
tx_context.block_context.block_info.use_kzg_da,
)?;
let gas = tx_resources.to_gas_vector_with_context(tx_context)?;

// 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 {
Expand Down
2 changes: 2 additions & 0 deletions crates/blockifier/src/test_utils/struct_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,12 @@ impl TransactionResources {
&self,
block_context: &BlockContext,
fee_type: &FeeType,
include_l2_gas: bool,
) -> TransactionFeeResult<Fee> {
let gas_vector = self.to_gas_vector(
&block_context.versioned_constants,
block_context.block_info.use_kzg_da,
include_l2_gas,
)?;
Ok(get_fee_by_gas_vector(&block_context.block_info, gas_vector, fee_type))
}
Expand Down
14 changes: 12 additions & 2 deletions crates/blockifier/src/transaction/account_transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -932,14 +932,19 @@ fn test_max_fee_to_max_steps_conversion(
nonce: nonce_manager.next(account_address),
});
let tx_context1 = Arc::new(block_context.to_tx_context(&account_tx1));
let has_l2_gas_bounds = tx_context1.tx_info.has_l2_gas_bounds();
let execution_context1 = EntryPointExecutionContext::new_invoke(tx_context1, true);
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.receipt.resources.vm_resources.n_steps;
let gas_used_vector1 = tx_execution_info1
.receipt
.resources
.to_gas_vector(&block_context.versioned_constants, block_context.block_info.use_kzg_da)
.to_gas_vector(
&block_context.versioned_constants,
block_context.block_info.use_kzg_da,
has_l2_gas_bounds,
)
.unwrap();

// Second invocation of `with_arg` gets twice the pre-calculated actual fee as max_fee.
Expand All @@ -952,14 +957,19 @@ fn test_max_fee_to_max_steps_conversion(
nonce: nonce_manager.next(account_address),
});
let tx_context2 = Arc::new(block_context.to_tx_context(&account_tx2));
let has_l2_gas_bounds = tx_context2.tx_info.has_l2_gas_bounds();
let execution_context2 = EntryPointExecutionContext::new_invoke(tx_context2, true);
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.receipt.resources.vm_resources.n_steps;
let gas_used_vector2 = tx_execution_info2
.receipt
.resources
.to_gas_vector(&block_context.versioned_constants, block_context.block_info.use_kzg_da)
.to_gas_vector(
&block_context.versioned_constants,
block_context.block_info.use_kzg_da,
has_l2_gas_bounds,
)
.unwrap();

// Test that steps limit doubles as max_fee doubles, but actual consumed steps and fee remains.
Expand Down
30 changes: 26 additions & 4 deletions crates/blockifier/src/transaction/execution_flavors_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,17 @@ fn check_gas_and_fee(
expected_actual_gas: u64,
expected_actual_fee: Fee,
expected_cost_of_resources: Fee,
include_l2_gas: bool,
) {
assert_eq!(
tx_execution_info
.receipt
.resources
.to_gas_vector(&block_context.versioned_constants, block_context.block_info.use_kzg_da)
.to_gas_vector(
&block_context.versioned_constants,
block_context.block_info.use_kzg_da,
include_l2_gas
)
.unwrap()
.l1_gas,
expected_actual_gas.into()
Expand All @@ -139,7 +144,11 @@ fn check_gas_and_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.receipt.resources.calculate_tx_fee(block_context, fee_type).unwrap(),
tx_execution_info
.receipt
.resources
.calculate_tx_fee(block_context, fee_type, include_l2_gas)
.unwrap(),
expected_cost_of_resources
);
}
Expand Down Expand Up @@ -171,6 +180,7 @@ fn test_simulate_validate_charge_fee_pre_validate(
// The max resource bounds fixture is not used here because this function already has the
// maximum number of arguments.
let resource_bounds = l1_resource_bounds(MAX_L1_GAS_AMOUNT, MAX_L1_GAS_PRICE);
let has_l2_gas = resource_bounds.has_l2_gas();
let gas_price = block_context.block_info.gas_prices.get_l1_gas_price_by_fee_type(&fee_type);
let FlavorTestInitialState {
mut state,
Expand Down Expand Up @@ -238,6 +248,7 @@ fn test_simulate_validate_charge_fee_pre_validate(
actual_gas_used,
actual_fee,
actual_fee,
has_l2_gas,
);
} else {
nonce_manager.rollback(account_address);
Expand Down Expand Up @@ -282,6 +293,7 @@ fn test_simulate_validate_charge_fee_pre_validate(
actual_gas_used,
actual_fee,
actual_fee,
has_l2_gas,
);
} else {
nonce_manager.rollback(account_address);
Expand Down Expand Up @@ -322,6 +334,7 @@ fn test_simulate_validate_charge_fee_pre_validate(
actual_gas_used,
actual_fee,
actual_fee,
has_l2_gas,
);
} else {
nonce_manager.rollback(account_address);
Expand Down Expand Up @@ -351,6 +364,7 @@ fn test_simulate_validate_charge_fee_fail_validate(
#[case] fee_type: FeeType,
max_resource_bounds: ValidResourceBounds,
) {
let has_l2_gas = max_resource_bounds.has_l2_gas();
let block_context = BlockContext::create_for_account_testing();
let max_fee = Fee(MAX_FEE);

Expand Down Expand Up @@ -391,6 +405,7 @@ fn test_simulate_validate_charge_fee_fail_validate(
actual_gas_used,
actual_fee,
actual_fee,
has_l2_gas,
);
} else {
assert!(
Expand All @@ -413,6 +428,7 @@ fn test_simulate_validate_charge_fee_mid_execution(
#[case] fee_type: FeeType,
max_resource_bounds: ValidResourceBounds,
) {
let has_l2_gas = max_resource_bounds.has_l2_gas();
let block_context = BlockContext::create_for_account_testing();
let chain_info = &block_context.chain_info;
let gas_price = block_context.block_info.gas_prices.get_l1_gas_price_by_fee_type(&fee_type);
Expand Down Expand Up @@ -462,6 +478,7 @@ fn test_simulate_validate_charge_fee_mid_execution(
revert_gas_used,
revert_fee,
revert_fee,
has_l2_gas,
);
let current_balance = check_balance(
current_balance,
Expand Down Expand Up @@ -513,6 +530,7 @@ fn test_simulate_validate_charge_fee_mid_execution(
// 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 },
has_l2_gas,
);
let current_balance =
check_balance(current_balance, &state, account_address, chain_info, &fee_type, charge_fee);
Expand Down Expand Up @@ -553,10 +571,10 @@ fn test_simulate_validate_charge_fee_mid_execution(
block_limit_gas,
block_limit_fee,
block_limit_fee,
has_l2_gas,
);
check_balance(current_balance, &state, account_address, chain_info, &fee_type, charge_fee);
}

#[rstest]
#[case(TransactionVersion::ONE, FeeType::Eth, true)]
#[case(TransactionVersion::THREE, FeeType::Strk, false)]
Expand Down Expand Up @@ -614,9 +632,11 @@ fn test_simulate_validate_charge_fee_post_execution(
validate,
&fee_type,
);
let resource_bounds = l1_resource_bounds(just_not_enough_gas_bound, gas_price.into());
let has_l2_gas = resource_bounds.has_l2_gas();
let tx_execution_info = account_invoke_tx(invoke_tx_args! {
max_fee: just_not_enough_fee_bound,
resource_bounds: l1_resource_bounds(just_not_enough_gas_bound, gas_price.into()),
resource_bounds,
calldata: recurse_calldata(test_contract_address, false, 1000),
nonce: nonce_manager.next(account_address),
sender_address: account_address,
Expand All @@ -641,6 +661,7 @@ fn test_simulate_validate_charge_fee_post_execution(
if charge_fee { revert_gas_usage } else { unlimited_gas_used },
if charge_fee { just_not_enough_fee_bound } else { unlimited_fee },
if charge_fee { revert_fee } else { unlimited_fee },
has_l2_gas,
);
let current_balance =
check_balance(current_balance, &state, account_address, chain_info, &fee_type, charge_fee);
Expand Down Expand Up @@ -706,6 +727,7 @@ fn test_simulate_validate_charge_fee_post_execution(
if charge_fee { fail_actual_gas } else { success_actual_gas },
actual_fee,
if charge_fee { fail_actual_fee } else { actual_fee },
has_l2_gas,
);
check_balance(
current_balance,
Expand Down
22 changes: 21 additions & 1 deletion crates/blockifier/src/transaction/objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use strum_macros::EnumIter;

use crate::abi::constants as abi_constants;
use crate::blockifier::block::{BlockInfo, GasPrices};
use crate::context::TransactionContext;
use crate::execution::call_info::{CallInfo, ExecutionSummary, MessageL1CostInfo, OrderedEvent};
use crate::fee::actual_cost::TransactionReceipt;
use crate::fee::eth_gas_constants;
Expand Down Expand Up @@ -108,6 +109,13 @@ impl TransactionInfo {
TransactionInfo::Deprecated(context) => context.max_fee != Fee(0),
}
}

pub fn has_l2_gas_bounds(&self) -> bool {
match self {
TransactionInfo::Current(context) => context.resource_bounds.has_l2_gas(),
TransactionInfo::Deprecated(_) => false,
}
}
}

impl HasRelatedFeeType for TransactionInfo {
Expand Down Expand Up @@ -489,15 +497,27 @@ impl TransactionResources {
&self,
versioned_constants: &VersionedConstants,
use_kzg_da: bool,
include_l2_gas: bool,
) -> TransactionFeeResult<GasVector> {
Ok(self.starknet_resources.to_gas_vector(versioned_constants, use_kzg_da, false)
Ok(self.starknet_resources.to_gas_vector(versioned_constants, use_kzg_da, include_l2_gas)
+ calculate_l1_gas_by_vm_usage(
versioned_constants,
&self.vm_resources,
self.n_reverted_steps,
)?)
}

pub fn to_gas_vector_with_context(
&self,
tx_context: &TransactionContext,
) -> TransactionFeeResult<GasVector> {
self.to_gas_vector(
&tx_context.block_context.versioned_constants,
tx_context.block_context.block_info.use_kzg_da,
tx_context.tx_info.has_l2_gas_bounds(),
)
}

pub fn to_resources_mapping(
&self,
versioned_constants: &VersionedConstants,
Expand Down
6 changes: 5 additions & 1 deletion crates/blockifier/src/transaction/post_execution_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,11 @@ fn test_revert_on_resource_overuse(
let actual_gas_usage: u64 = execution_info_measure
.receipt
.resources
.to_gas_vector(&block_context.versioned_constants, block_context.block_info.use_kzg_da)
.to_gas_vector(
&block_context.versioned_constants,
block_context.block_info.use_kzg_da,
false, // TODO(Nimrod): Derive this value from `max_resource_bounds`.
)
.unwrap()
.l1_gas
.try_into()
Expand Down
49 changes: 35 additions & 14 deletions crates/blockifier/src/transaction/transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,8 +475,11 @@ fn test_invoke_tx(

// Build expected fee transfer call info.
let fee_type = &tx_context.tx_info.fee_type();
let expected_actual_fee =
actual_execution_info.receipt.resources.calculate_tx_fee(block_context, fee_type).unwrap();
let expected_actual_fee = actual_execution_info
.receipt
.resources
.calculate_tx_fee(block_context, fee_type, tx_context.tx_info.has_l2_gas_bounds())
.unwrap();
let expected_fee_transfer_call_info = expected_fee_transfer_call_info(
&tx_context,
sender_address,
Expand Down Expand Up @@ -507,7 +510,11 @@ fn test_invoke_tx(
);

let total_gas = expected_actual_resources
.to_gas_vector(&block_context.versioned_constants, block_context.block_info.use_kzg_da)
.to_gas_vector(
&block_context.versioned_constants,
block_context.block_info.use_kzg_da,
tx_context.tx_info.has_l2_gas_bounds(),
)
.unwrap();

let expected_execution_info = TransactionExecutionInfo {
Expand Down Expand Up @@ -1198,8 +1205,11 @@ 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_actual_fee = actual_execution_info
.receipt
.resources
.calculate_tx_fee(block_context, fee_type, tx_context.tx_info.has_l2_gas_bounds())
.unwrap();
let expected_fee_transfer_call_info = expected_fee_transfer_call_info(
tx_context,
sender_address,
Expand Down Expand Up @@ -1228,8 +1238,9 @@ fn test_declare_tx(
use_kzg_da,
);

let expected_total_gas =
expected_actual_resources.to_gas_vector(versioned_constants, use_kzg_da).unwrap();
let expected_total_gas = expected_actual_resources
.to_gas_vector(versioned_constants, use_kzg_da, tx_context.tx_info.has_l2_gas_bounds())
.unwrap();

let expected_execution_info = TransactionExecutionInfo {
validate_call_info: expected_validate_call_info,
Expand Down Expand Up @@ -1359,8 +1370,11 @@ fn test_deploy_account_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_actual_fee = actual_execution_info
.receipt
.resources
.calculate_tx_fee(block_context, fee_type, tx_context.tx_info.has_l2_gas_bounds())
.unwrap();
let expected_fee_transfer_call_info = expected_fee_transfer_call_info(
tx_context,
deployed_account_address,
Expand Down Expand Up @@ -1397,7 +1411,11 @@ fn test_deploy_account_tx(
);

let expected_total_gas = actual_resources
.to_gas_vector(&block_context.versioned_constants, block_context.block_info.use_kzg_da)
.to_gas_vector(
&block_context.versioned_constants,
block_context.block_info.use_kzg_da,
tx_context.tx_info.has_l2_gas_bounds(),
)
.unwrap();

let expected_execution_info = TransactionExecutionInfo {
Expand Down Expand Up @@ -1920,7 +1938,7 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) {
);

let total_gas = expected_tx_resources
.to_gas_vector(versioned_constants, block_context.block_info.use_kzg_da)
.to_gas_vector(versioned_constants, block_context.block_info.use_kzg_da, false)
.unwrap();

// Build the expected execution info.
Expand Down Expand Up @@ -1954,9 +1972,12 @@ fn test_l1_handler(#[values(false, true)] use_kzg_da: bool) {
let tx_no_fee = L1HandlerTransaction::create_for_testing(Fee(0), contract_address);
let error = tx_no_fee.execute(state, block_context, true, true).unwrap_err();
// Today, we check that the paid_fee is positive, no matter what was the actual fee.
let expected_actual_fee =
(expected_execution_info.receipt.resources.calculate_tx_fee(block_context, &FeeType::Eth))
.unwrap();
let expected_actual_fee = (expected_execution_info.receipt.resources.calculate_tx_fee(
block_context,
&FeeType::Eth,
false,
))
.unwrap();
assert_matches!(
error,
TransactionExecutionError::TransactionFeeError(
Expand Down
7 changes: 7 additions & 0 deletions crates/starknet_api/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,13 @@ impl ValidResourceBounds {
}
}
}

pub fn has_l2_gas(&self) -> bool {
match self {
ValidResourceBounds::L1Gas(_) => false,
ValidResourceBounds::AllResources(_) => true,
}
}
}

#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Hash, Ord, PartialOrd, Serialize)]
Expand Down
Loading