Skip to content

Commit

Permalink
refactor(fee): use valid resource bounds in blockifier
Browse files Browse the repository at this point in the history
  • Loading branch information
nimrod-starkware committed Aug 21, 2024
1 parent 295e9ff commit f672171
Show file tree
Hide file tree
Showing 14 changed files with 65 additions and 83 deletions.
4 changes: 2 additions & 2 deletions crates/blockifier/src/concurrency/fee_utils_test.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use num_bigint::BigUint;
use rstest::rstest;
use starknet_api::felt;
use starknet_api::transaction::{DeprecatedResourceBoundsMapping, Fee};
use starknet_api::transaction::{Fee, ValidResourceBounds};
use starknet_types_core::felt::Felt;

use crate::concurrency::fee_utils::{add_fee_to_sequencer_balance, fill_sequencer_balance_reads};
Expand All @@ -19,7 +19,7 @@ use crate::transaction::test_utils::{account_invoke_tx, block_context, max_resou
#[rstest]
pub fn test_fill_sequencer_balance_reads(
block_context: BlockContext,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
#[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] erc20_version: CairoVersion,
) {
let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1);
Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/concurrency/versioned_state_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use starknet_api::core::{
Nonce,
PatriciaKey,
};
use starknet_api::transaction::{Calldata, ContractAddressSalt, DeprecatedResourceBoundsMapping};
use starknet_api::transaction::{Calldata, ContractAddressSalt, ValidResourceBounds};
use starknet_api::{calldata, class_hash, contract_address, felt, patricia_key};

use crate::abi::abi_utils::{get_fee_token_var_address, get_storage_var_address};
Expand Down Expand Up @@ -201,7 +201,7 @@ fn test_versioned_state_proxy() {

#[rstest]
// Test parallel execution of two transactions that use the same versioned state.
fn test_run_parallel_txs(max_resource_bounds: DeprecatedResourceBoundsMapping) {
fn test_run_parallel_txs(max_resource_bounds: ValidResourceBounds) {
let block_context = BlockContext::create_for_account_testing();
let chain_info = &block_context.chain_info;
let zero_bounds = true;
Expand Down
10 changes: 5 additions & 5 deletions crates/blockifier/src/concurrency/worker_logic_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use rstest::rstest;
use starknet_api::core::{ContractAddress, Nonce, PatriciaKey};
use starknet_api::transaction::{
ContractAddressSalt,
DeprecatedResourceBoundsMapping,
Fee,
TransactionVersion,
ValidResourceBounds,
};
use starknet_api::{contract_address, felt, patricia_key};
use starknet_types_core::felt::Felt;
Expand Down Expand Up @@ -256,7 +256,7 @@ fn test_commit_tx_when_sender_is_sequencer() {
}

#[rstest]
fn test_worker_execute(max_resource_bounds: DeprecatedResourceBoundsMapping) {
fn test_worker_execute(max_resource_bounds: ValidResourceBounds) {
// Settings.
let block_context = BlockContext::create_for_account_testing();
let account_contract = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1);
Expand Down Expand Up @@ -430,7 +430,7 @@ fn test_worker_execute(max_resource_bounds: DeprecatedResourceBoundsMapping) {
}

#[rstest]
fn test_worker_validate(max_resource_bounds: DeprecatedResourceBoundsMapping) {
fn test_worker_validate(max_resource_bounds: ValidResourceBounds) {
// Settings.
let block_context = BlockContext::create_for_account_testing();
let account_contract = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1);
Expand Down Expand Up @@ -537,7 +537,7 @@ fn test_worker_validate(max_resource_bounds: DeprecatedResourceBoundsMapping) {
#[case::declare_cairo1(CairoVersion::Cairo1, TransactionVersion::THREE)]
fn test_deploy_before_declare(
max_fee: Fee,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
#[case] cairo_version: CairoVersion,
#[case] version: TransactionVersion,
) {
Expand Down Expand Up @@ -629,7 +629,7 @@ fn test_deploy_before_declare(
}

#[rstest]
fn test_worker_commit_phase(max_resource_bounds: DeprecatedResourceBoundsMapping) {
fn test_worker_commit_phase(max_resource_bounds: ValidResourceBounds) {
// Settings.
let block_context = BlockContext::create_for_account_testing();
let account_contract = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1);
Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/execution/stack_trace_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ use starknet_api::core::{calculate_contract_address, Nonce};
use starknet_api::transaction::{
Calldata,
ContractAddressSalt,
DeprecatedResourceBoundsMapping,
Fee,
TransactionSignature,
TransactionVersion,
ValidResourceBounds,
};
use starknet_api::{calldata, felt};

Expand Down Expand Up @@ -599,7 +599,7 @@ An ASSERT_EQ instruction failed: 1 != 0.
/// point selector).
fn test_contract_ctor_frame_stack_trace(
block_context: BlockContext,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
#[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion,
) {
let chain_info = &block_context.chain_info;
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
@@ -1,5 +1,5 @@
use rstest::{fixture, rstest};
use starknet_api::transaction::{DeprecatedResourceBoundsMapping, L2ToL1Payload};
use starknet_api::transaction::{L2ToL1Payload, ValidResourceBounds};
use starknet_types_core::felt::Felt;

use crate::context::BlockContext;
Expand Down Expand Up @@ -286,7 +286,7 @@ fn test_calculate_tx_gas_usage_basic<'a>(#[values(false, true)] use_kzg_da: bool
// resources are taken into account).
#[rstest]
fn test_calculate_tx_gas_usage(
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
#[values(false, true)] use_kzg_da: bool,
) {
let account_cairo_version = CairoVersion::Cairo0;
Expand Down
15 changes: 5 additions & 10 deletions crates/blockifier/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ use starknet_api::state::StorageKey;
use starknet_api::transaction::{
Calldata,
ContractAddressSalt,
DeprecatedResourceBoundsMapping,
Resource,
ResourceBounds,
TransactionVersion,
ValidResourceBounds,
};
use starknet_api::{contract_address, felt, patricia_key};
use starknet_types_core::felt::Felt;
Expand Down Expand Up @@ -211,14 +210,10 @@ pub fn trivial_external_entry_point_with_address(
}
}

fn default_testing_resource_bounds() -> DeprecatedResourceBoundsMapping {
DeprecatedResourceBoundsMapping::try_from(vec![
(Resource::L1Gas, ResourceBounds { max_amount: 0, max_price_per_unit: 1 }),
// TODO(Dori, 1/2/2024): When fee market is developed, change the default price of
// L2 gas.
(Resource::L2Gas, ResourceBounds { max_amount: 0, max_price_per_unit: 0 }),
])
.unwrap()
fn default_testing_resource_bounds() -> ValidResourceBounds {
// TODO(Dori, 1/2/2024): When fee market is developed, change the default price of
// L2 gas.
ValidResourceBounds::L1Gas(ResourceBounds { max_amount: 0, max_price_per_unit: 1 })
}

#[macro_export]
Expand Down
6 changes: 3 additions & 3 deletions crates/blockifier/src/test_utils/declare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ use starknet_api::transaction::{
DeclareTransactionV0V1,
DeclareTransactionV2,
DeclareTransactionV3,
DeprecatedResourceBoundsMapping,
Fee,
PaymasterData,
Tip,
TransactionHash,
TransactionSignature,
TransactionVersion,
ValidResourceBounds,
};

use crate::execution::contract_class::ClassInfo;
Expand All @@ -25,7 +25,7 @@ pub struct DeclareTxArgs {
pub signature: TransactionSignature,
pub sender_address: ContractAddress,
pub version: TransactionVersion,
pub resource_bounds: DeprecatedResourceBoundsMapping,
pub resource_bounds: ValidResourceBounds,
pub tip: Tip,
pub nonce_data_availability_mode: DataAvailabilityMode,
pub fee_data_availability_mode: DataAvailabilityMode,
Expand Down Expand Up @@ -108,7 +108,7 @@ pub fn declare_tx(declare_tx_args: DeclareTxArgs, class_info: ClassInfo) -> Acco
starknet_api::transaction::DeclareTransaction::V3(DeclareTransactionV3 {
signature: declare_tx_args.signature,
sender_address: declare_tx_args.sender_address,
resource_bounds: declare_tx_args.resource_bounds.try_into().expect("todo"),
resource_bounds: declare_tx_args.resource_bounds,
tip: declare_tx_args.tip,
nonce_data_availability_mode: declare_tx_args.nonce_data_availability_mode,
fee_data_availability_mode: declare_tx_args.fee_data_availability_mode,
Expand Down
6 changes: 3 additions & 3 deletions crates/blockifier/src/test_utils/deploy_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ use starknet_api::transaction::{
ContractAddressSalt,
DeployAccountTransactionV1,
DeployAccountTransactionV3,
DeprecatedResourceBoundsMapping,
Fee,
PaymasterData,
Tip,
TransactionHash,
TransactionSignature,
TransactionVersion,
ValidResourceBounds,
};

use crate::test_utils::{default_testing_resource_bounds, NonceManager};
Expand All @@ -23,7 +23,7 @@ pub struct DeployAccountTxArgs {
pub signature: TransactionSignature,
pub deployer_address: ContractAddress,
pub version: TransactionVersion,
pub resource_bounds: DeprecatedResourceBoundsMapping,
pub resource_bounds: ValidResourceBounds,
pub tip: Tip,
pub nonce_data_availability_mode: DataAvailabilityMode,
pub fee_data_availability_mode: DataAvailabilityMode,
Expand Down Expand Up @@ -96,7 +96,7 @@ pub fn deploy_account_tx(
} else if deploy_tx_args.version == TransactionVersion::THREE {
starknet_api::transaction::DeployAccountTransaction::V3(DeployAccountTransactionV3 {
signature: deploy_tx_args.signature,
resource_bounds: deploy_tx_args.resource_bounds.try_into().expect("todo"),
resource_bounds: deploy_tx_args.resource_bounds,
tip: deploy_tx_args.tip,
nonce_data_availability_mode: deploy_tx_args.nonce_data_availability_mode,
fee_data_availability_mode: deploy_tx_args.fee_data_availability_mode,
Expand Down
6 changes: 3 additions & 3 deletions crates/blockifier/src/test_utils/invoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use starknet_api::data_availability::DataAvailabilityMode;
use starknet_api::transaction::{
AccountDeploymentData,
Calldata,
DeprecatedResourceBoundsMapping,
Fee,
InvokeTransactionV0,
InvokeTransactionV1,
Expand All @@ -14,6 +13,7 @@ use starknet_api::transaction::{
TransactionHash,
TransactionSignature,
TransactionVersion,
ValidResourceBounds,
};

use crate::abi::abi_utils::selector_from_name;
Expand All @@ -28,7 +28,7 @@ pub struct InvokeTxArgs {
pub sender_address: ContractAddress,
pub calldata: Calldata,
pub version: TransactionVersion,
pub resource_bounds: DeprecatedResourceBoundsMapping,
pub resource_bounds: ValidResourceBounds,
pub tip: Tip,
pub nonce_data_availability_mode: DataAvailabilityMode,
pub fee_data_availability_mode: DataAvailabilityMode,
Expand Down Expand Up @@ -96,7 +96,7 @@ pub fn invoke_tx(invoke_args: InvokeTxArgs) -> InvokeTransaction {
})
} else if invoke_args.version == TransactionVersion::THREE {
starknet_api::transaction::InvokeTransaction::V3(InvokeTransactionV3 {
resource_bounds: invoke_args.resource_bounds.try_into().expect("todo"),
resource_bounds: invoke_args.resource_bounds,
calldata: invoke_args.calldata,
sender_address: invoke_args.sender_address,
nonce: invoke_args.nonce,
Expand Down
33 changes: 15 additions & 18 deletions crates/blockifier/src/transaction/account_transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ use starknet_api::transaction::{
Calldata,
ContractAddressSalt,
DeclareTransactionV2,
DeprecatedResourceBoundsMapping,
Fee,
TransactionHash,
TransactionVersion,
ValidResourceBounds,
};
use starknet_api::{calldata, class_hash, contract_address, felt, patricia_key};
use starknet_types_core::felt::Felt;
Expand Down Expand Up @@ -80,7 +80,7 @@ use crate::{
};

#[rstest]
fn test_circuit(block_context: BlockContext, max_resource_bounds: DeprecatedResourceBoundsMapping) {
fn test_circuit(block_context: BlockContext, max_resource_bounds: ValidResourceBounds) {
let test_contract = FeatureContract::TestContract(CairoVersion::Cairo1);
let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1);
let chain_info = &block_context.chain_info;
Expand Down Expand Up @@ -114,10 +114,7 @@ fn test_circuit(block_context: BlockContext, max_resource_bounds: DeprecatedReso
}

#[rstest]
fn test_rc96_holes(
block_context: BlockContext,
max_resource_bounds: DeprecatedResourceBoundsMapping,
) {
fn test_rc96_holes(block_context: BlockContext, max_resource_bounds: ValidResourceBounds) {
let test_contract = FeatureContract::TestContract(CairoVersion::Cairo1);
let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1);
let chain_info = &block_context.chain_info;
Expand Down Expand Up @@ -208,7 +205,7 @@ fn test_enforce_fee_false_works(block_context: BlockContext, #[case] version: Tr
fn test_account_flow_test(
block_context: BlockContext,
max_fee: Fee,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
#[values(TransactionVersion::ZERO, TransactionVersion::ONE, TransactionVersion::THREE)]
tx_version: TransactionVersion,
#[values(true, false)] only_query: bool,
Expand Down Expand Up @@ -240,7 +237,7 @@ fn test_account_flow_test(
fn test_invoke_tx_from_non_deployed_account(
block_context: BlockContext,
max_fee: Fee,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
#[case] tx_version: TransactionVersion,
) {
let TestInitData { mut state, account_address, contract_address: _, mut nonce_manager } =
Expand Down Expand Up @@ -289,7 +286,7 @@ fn test_infinite_recursion(
#[values(true, false)] success: bool,
#[values(true, false)] normal_recurse: bool,
mut block_context: BlockContext,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
) {
// Limit the number of execution steps (so we quickly hit the limit).
block_context.versioned_constants.invoke_tx_max_n_steps = 4100;
Expand Down Expand Up @@ -344,7 +341,7 @@ fn test_infinite_recursion(
fn test_max_fee_limit_validate(
block_context: BlockContext,
#[case] version: TransactionVersion,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
) {
let chain_info = &block_context.chain_info;
let TestInitData { mut state, account_address, contract_address, mut nonce_manager } =
Expand Down Expand Up @@ -449,7 +446,7 @@ fn test_recursion_depth_exceeded(
#[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion,
block_context: BlockContext,
max_fee: Fee,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
) {
let TestInitData { mut state, account_address, contract_address, mut nonce_manager } =
create_test_init_data(&block_context.chain_info, cairo_version);
Expand Down Expand Up @@ -693,7 +690,7 @@ fn recursive_function_calldata(
#[case(TransactionVersion::THREE)]
fn test_reverted_reach_steps_limit(
max_fee: Fee,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
mut block_context: BlockContext,
#[case] version: TransactionVersion,
#[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion,
Expand Down Expand Up @@ -801,7 +798,7 @@ fn test_reverted_reach_steps_limit(
/// asserts false. We test deltas between consecutive depths, and further depths.
fn test_n_reverted_steps(
block_context: BlockContext,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
#[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion,
) {
let TestInitData { mut state, account_address, contract_address, mut nonce_manager } =
Expand Down Expand Up @@ -982,7 +979,7 @@ fn test_max_fee_to_max_steps_conversion(
/// recorded and max_fee is charged.
fn test_insufficient_max_fee_reverts(
block_context: BlockContext,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
#[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion,
) {
let TestInitData { mut state, account_address, contract_address, mut nonce_manager } =
Expand Down Expand Up @@ -1049,7 +1046,7 @@ fn test_insufficient_max_fee_reverts(

#[rstest]
fn test_deploy_account_constructor_storage_write(
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
block_context: BlockContext,
#[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion,
) {
Expand Down Expand Up @@ -1093,7 +1090,7 @@ fn test_deploy_account_constructor_storage_write(
fn test_count_actual_storage_changes(
max_fee: Fee,
block_context: BlockContext,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
#[case] version: TransactionVersion,
#[case] fee_type: FeeType,
#[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion,
Expand Down Expand Up @@ -1274,7 +1271,7 @@ fn test_count_actual_storage_changes(
#[case::tx_version_3(TransactionVersion::THREE)]
fn test_concurrency_execute_fee_transfer(
max_fee: Fee,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
#[case] version: TransactionVersion,
) {
// TODO(Meshi, 01/06/2024): make the test so it will include changes in
Expand Down Expand Up @@ -1374,7 +1371,7 @@ fn test_concurrency_execute_fee_transfer(
#[case::tx_version_3(TransactionVersion::THREE)]
fn test_concurrent_fee_transfer_when_sender_is_sequencer(
max_fee: Fee,
max_resource_bounds: DeprecatedResourceBoundsMapping,
max_resource_bounds: ValidResourceBounds,
#[case] version: TransactionVersion,
) {
let mut block_context = BlockContext::create_for_account_testing();
Expand Down
Loading

0 comments on commit f672171

Please sign in to comment.