diff --git a/crates/blockifier/src/concurrency.rs b/crates/blockifier/src/concurrency.rs index 37ba6dc68d..f898b46b09 100644 --- a/crates/blockifier/src/concurrency.rs +++ b/crates/blockifier/src/concurrency.rs @@ -1,3 +1,4 @@ +pub mod fee_utils; pub mod scheduler; #[cfg(any(feature = "testing", test))] pub mod test_utils; diff --git a/crates/blockifier/src/concurrency/fee_utils.rs b/crates/blockifier/src/concurrency/fee_utils.rs new file mode 100644 index 0000000000..85d01871ca --- /dev/null +++ b/crates/blockifier/src/concurrency/fee_utils.rs @@ -0,0 +1,30 @@ +use starknet_api::hash::StarkFelt; + +use crate::execution::call_info::CallInfo; +#[cfg(test)] +#[path = "fee_utils_test.rs"] +mod test; + +// We read account balance (sender), and sequencer balance (recipient). The balance is of type +// `Uint256`, consist of two felts (lsb, msb). Hence, storage read values = +// [account_balance, 0, sequencer_balance, 0] +const STORAGE_READ_SEQUENCER_BALANCE_INDICES: (usize, usize) = (2, 3); + +// Completes the fee transfer execution by fixing the call info to have the correct sequencer +// balance. In concurrency mode, the fee transfer is executed with a false (constant) sequencer +// balance. This affects the call info. +pub fn fill_sequencer_balance_reads( + fee_transfer_call_info: &mut CallInfo, + sequencer_balance_low: StarkFelt, + sequencer_balance_high: StarkFelt, +) { + let storage_read_values = &mut fee_transfer_call_info.storage_read_values; + assert_eq!(storage_read_values.len(), 4, "Storage read values should have 4 elements"); + + let (low_index, high_index) = STORAGE_READ_SEQUENCER_BALANCE_INDICES; + for index in [low_index, high_index] { + assert_eq!(storage_read_values[index], StarkFelt::ZERO, "Sequencer balance should be zero"); + } + storage_read_values[low_index] = sequencer_balance_low; + storage_read_values[high_index] = sequencer_balance_high; +} diff --git a/crates/blockifier/src/concurrency/fee_utils_test.rs b/crates/blockifier/src/concurrency/fee_utils_test.rs new file mode 100644 index 0000000000..af581716b5 --- /dev/null +++ b/crates/blockifier/src/concurrency/fee_utils_test.rs @@ -0,0 +1,44 @@ +use rstest::rstest; +use starknet_api::hash::StarkFelt; +use starknet_api::transaction::TransactionVersion; + +use crate::concurrency::fee_utils::fill_sequencer_balance_reads; +use crate::concurrency::test_utils::create_fee_transfer_call_info; +use crate::context::BlockContext; +use crate::invoke_tx_args; +use crate::test_utils::contracts::FeatureContract; +use crate::test_utils::initial_test_state::{fund_account, test_state}; +use crate::test_utils::{ + create_trivial_calldata, CairoVersion, BALANCE, MAX_L1_GAS_AMOUNT, MAX_L1_GAS_PRICE, +}; +use crate::transaction::test_utils::{account_invoke_tx, block_context, l1_resource_bounds}; + +#[rstest] +pub fn test_fill_sequencer_balance_reads(block_context: BlockContext) { + let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1); + let account_tx = account_invoke_tx(invoke_tx_args! { + sender_address: account.get_instance_address(0), + calldata: create_trivial_calldata(account.get_instance_address(0)), + resource_bounds: l1_resource_bounds(MAX_L1_GAS_AMOUNT, MAX_L1_GAS_PRICE), + version: TransactionVersion::THREE + }); + let chain_info = &block_context.chain_info; + let state = &mut test_state(chain_info, BALANCE, &[(account, 1)]); + + let sequencer_balance = 100; + let sequencer_address = block_context.block_info.sequencer_address; + fund_account(chain_info, sequencer_address, sequencer_balance, &mut state.state); + + let mut concurrency_call_info = create_fee_transfer_call_info(state, &account_tx, true); + let call_info = create_fee_transfer_call_info(state, &account_tx, false); + + assert_ne!(concurrency_call_info, call_info); + + fill_sequencer_balance_reads( + &mut concurrency_call_info, + StarkFelt::from(sequencer_balance), + StarkFelt::ZERO, + ); + + assert_eq!(concurrency_call_info, call_info); +} diff --git a/crates/blockifier/src/concurrency/test_utils.rs b/crates/blockifier/src/concurrency/test_utils.rs index f64d1b2a75..ee92fb3620 100644 --- a/crates/blockifier/src/concurrency/test_utils.rs +++ b/crates/blockifier/src/concurrency/test_utils.rs @@ -1,5 +1,11 @@ use crate::concurrency::versioned_state_proxy::{ThreadSafeVersionedState, VersionedState}; +use crate::context::BlockContext; +use crate::execution::call_info::CallInfo; +use crate::state::cached_state::CachedState; +use crate::state::state_api::StateReader; use crate::test_utils::dict_state_reader::DictStateReader; +use crate::transaction::account_transaction::AccountTransaction; +use crate::transaction::transactions::ExecutableTransaction; #[macro_export] macro_rules! default_scheduler { @@ -35,3 +41,23 @@ pub fn safe_versioned_state_for_testing( ) -> ThreadSafeVersionedState { ThreadSafeVersionedState::new(VersionedState::new(block_state)) } + +// Note: this function does not mutate the state. +pub fn create_fee_transfer_call_info( + state: &mut CachedState, + account_tx: &AccountTransaction, + concurrency_mode: bool, +) -> CallInfo { + let block_context = + BlockContext::create_for_account_testing_with_concurrency_mode(concurrency_mode); + let mut transactional_state = CachedState::create_transactional(state); + let charge_fee = true; + let validate = true; + let execution_info = account_tx + .execute_raw(&mut transactional_state, &block_context, charge_fee, validate) + .unwrap(); + + let execution_info = execution_info.fee_transfer_call_info.unwrap(); + transactional_state.abort(); + execution_info +} diff --git a/crates/blockifier/src/fee/fee_utils.rs b/crates/blockifier/src/fee/fee_utils.rs index db5b16ac82..74f0be60c2 100644 --- a/crates/blockifier/src/fee/fee_utils.rs +++ b/crates/blockifier/src/fee/fee_utils.rs @@ -2,9 +2,12 @@ use std::collections::HashSet; use cairo_vm::vm::runners::cairo_runner::ExecutionResources; use starknet_api::hash::StarkFelt; +use starknet_api::state::StorageKey; use starknet_api::transaction::Fee; +use crate::abi::abi_utils::get_fee_token_var_address; use crate::abi::constants; +use crate::abi::sierra_types::next_storage_key; use crate::blockifier::block::BlockInfo; use crate::context::{BlockContext, TransactionContext}; use crate::state::state_api::StateReader; @@ -141,3 +144,11 @@ pub fn verify_can_pay_committed_bounds( }) } } + +pub fn get_sequencer_balance_keys(block_context: &BlockContext) -> (StorageKey, StorageKey) { + let sequencer_address = block_context.block_info.sequencer_address; + let sequencer_balance_key_low = get_fee_token_var_address(sequencer_address); + let sequencer_balance_key_high = next_storage_key(&sequencer_balance_key_low) + .expect("Cannot get sequencer balance high key."); + (sequencer_balance_key_low, sequencer_balance_key_high) +} diff --git a/crates/blockifier/src/transaction/account_transaction.rs b/crates/blockifier/src/transaction/account_transaction.rs index 64f171fa6a..469b91605d 100644 --- a/crates/blockifier/src/transaction/account_transaction.rs +++ b/crates/blockifier/src/transaction/account_transaction.rs @@ -7,15 +7,16 @@ use starknet_api::deprecated_contract_class::EntryPointType; use starknet_api::hash::StarkFelt; use starknet_api::transaction::{Calldata, Fee, ResourceBounds, TransactionVersion}; -use crate::abi::abi_utils::{get_fee_token_var_address, selector_from_name}; -use crate::abi::sierra_types::next_storage_key; +use crate::abi::abi_utils::selector_from_name; use crate::context::{BlockContext, TransactionContext}; use crate::execution::call_info::{CallInfo, Retdata}; use crate::execution::contract_class::ContractClass; use crate::execution::entry_point::{CallEntryPoint, CallType, EntryPointExecutionContext}; use crate::fee::actual_cost::TransactionReceipt; use crate::fee::fee_checks::{FeeCheckReportFields, PostExecutionReport}; -use crate::fee::fee_utils::{get_fee_by_gas_vector, verify_can_pay_committed_bounds}; +use crate::fee::fee_utils::{ + get_fee_by_gas_vector, get_sequencer_balance_keys, verify_can_pay_committed_bounds, +}; use crate::fee::gas_usage::{compute_discounted_gas_from_gas_vector, estimate_minimal_gas_vector}; use crate::retdata; use crate::state::cached_state::{CachedState, StateChanges, TransactionalState}; @@ -378,10 +379,8 @@ impl AccountTransaction { ) -> TransactionExecutionResult { let TransactionContext { block_context, tx_info } = tx_context.as_ref(); let fee_address = block_context.chain_info.fee_token_address(&tx_info.fee_type()); - let sequencer_address = block_context.block_info.sequencer_address; - let sequencer_balance_key_low = get_fee_token_var_address(sequencer_address); - let sequencer_balance_key_high = next_storage_key(&sequencer_balance_key_low) - .expect("Cannot get sequencer balance high key."); + let (sequencer_balance_key_low, sequencer_balance_key_high) = + get_sequencer_balance_keys(block_context); let mut transfer_state = CachedState::create_transactional(state); // Set the initial sequencer balance to avoid tarnishing the read-set of the transaction. diff --git a/crates/blockifier/src/transaction/account_transactions_test.rs b/crates/blockifier/src/transaction/account_transactions_test.rs index 8120050356..e4200a3b43 100644 --- a/crates/blockifier/src/transaction/account_transactions_test.rs +++ b/crates/blockifier/src/transaction/account_transactions_test.rs @@ -17,13 +17,12 @@ use starknet_api::{calldata, class_hash, contract_address, patricia_key, stark_f use crate::abi::abi_utils::{ get_fee_token_var_address, get_storage_var_address, selector_from_name, }; -use crate::abi::sierra_types::next_storage_key; use crate::context::BlockContext; use crate::execution::contract_class::{ContractClass, ContractClassV1}; use crate::execution::entry_point::EntryPointExecutionContext; use crate::execution::execution_utils::{felt_to_stark_felt, stark_felt_to_felt}; use crate::execution::syscalls::SyscallSelector; -use crate::fee::fee_utils::get_fee_by_gas_vector; +use crate::fee::fee_utils::{get_fee_by_gas_vector, get_sequencer_balance_keys}; use crate::fee::gas_usage::estimate_minimal_gas_vector; use crate::state::cached_state::{CachedState, StateChangesCount}; use crate::state::state_api::{State, StateReader}; @@ -1223,32 +1222,25 @@ fn test_count_actual_storage_changes( #[rstest] fn test_concurrency_execute_fee_transfer(#[values(FeeType::Eth, FeeType::Strk)] fee_type: FeeType) { - const STORAGE_WRITE_HIGH: u128 = 150; + // TODO(Meshi, 01/06/2024): make the test so it will include changes in + // sequencer_balance_key_high. const STORAGE_WRITE_LOW: u128 = 100; const STORAGE_READ_LOW: u128 = 50; let block_context = BlockContext::create_for_account_testing_with_concurrency_mode(true); - let empty_contract = FeatureContract::Empty(CairoVersion::Cairo1); let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1); + let account_tx = account_invoke_tx(invoke_tx_args! { + sender_address: account.get_instance_address(0), + calldata: create_trivial_calldata(account.get_instance_address(0)), + resource_bounds: l1_resource_bounds(MAX_L1_GAS_AMOUNT, MAX_L1_GAS_PRICE), + version: TransactionVersion::THREE + }); let chain_info = &block_context.chain_info; + let fee_token_address = block_context.chain_info.fee_token_address(&fee_type); let state = &mut test_state(chain_info, BALANCE, &[(account, 1)]); - let class_hash = empty_contract.get_class_hash(); - let class_info = calculate_class_info_for_testing(empty_contract.get_class()); - let sender_address = account.get_instance_address(0); - let account_tx = declare_tx( - declare_tx_args! { - sender_address, - version: TransactionVersion::THREE, - resource_bounds: l1_resource_bounds(MAX_L1_GAS_AMOUNT, MAX_L1_GAS_PRICE), - class_hash, - }, - class_info.clone(), - ); + let (sequencer_balance_key_low, sequencer_balance_key_high) = + get_sequencer_balance_keys(&block_context); - let fee_token_address = block_context.chain_info.fee_token_address(&fee_type); - let sequencer_address = block_context.block_info.sequencer_address; - let sequencer_balance_key_low = get_fee_token_var_address(sequencer_address); - let sequencer_balance_key_high = next_storage_key(&sequencer_balance_key_low).unwrap(); // Case 1: The transaction did not read form/ write to the sequenser balance before executing // fee transfer. let mut transactional_state = CachedState::create_transactional(state); @@ -1266,40 +1258,52 @@ fn test_concurrency_execute_fee_transfer(#[values(FeeType::Eth, FeeType::Strk)] // Case 2: The transaction read from and write to the sequenser balance before executing fee // transfer. + let transfer_calldata = create_calldata( + fee_token_address, + TRANSFER_ENTRY_POINT_NAME, + &[ + *block_context.block_info.sequencer_address.0.key(), + stark_felt!(STORAGE_WRITE_LOW), + stark_felt!(0_u8), + ], + ); + // Set the sequencer balance to a constant value to check that the read set did not changed. - fund_account(chain_info, sequencer_address, STORAGE_READ_LOW, &mut state.state); + fund_account( + chain_info, + block_context.block_info.sequencer_address, + STORAGE_READ_LOW, + &mut state.state, + ); let mut transactional_state = CachedState::create_transactional(state); - // Set the sequencer balance write set to a constant value. - // Note that it is enough to set the storage_write as execute_raw will update the - // storage_initial_values. - for (seq_key, value) in [ - (sequencer_balance_key_low, STORAGE_WRITE_LOW), - (sequencer_balance_key_high, STORAGE_WRITE_HIGH), - ] { - transactional_state.set_storage_at(fee_token_address, seq_key, stark_felt!(value)).unwrap(); - } + // Invokes transfer to the sequencer. + let account_tx = account_invoke_tx(invoke_tx_args! { + sender_address: account.get_instance_address(0), + calldata: transfer_calldata, + resource_bounds: l1_resource_bounds(MAX_L1_GAS_AMOUNT, MAX_L1_GAS_PRICE), + version: TransactionVersion::THREE + }); + + account_tx.execute_raw(&mut transactional_state, &block_context, true, true).unwrap(); - account_tx.execute_raw(&mut transactional_state, &block_context, true, false).unwrap(); // Check that the sequencer balance was not changed. - let storage_write = transactional_state.cache.borrow().writes.storage.clone(); - let storage_initial_values = transactional_state.cache.borrow().initial_reads.storage.clone(); + let storage_writes = transactional_state.cache.borrow().writes.storage.clone(); + let storage_initial_reads = transactional_state.cache.borrow().initial_reads.storage.clone(); for (seq_write_val, expexted_write_val) in [ ( - storage_write.get(&(fee_token_address, sequencer_balance_key_low)), - stark_felt!(STORAGE_WRITE_LOW), + storage_writes.get(&(fee_token_address, sequencer_balance_key_low)), + // Balance after `execute` and without the fee transfer. + stark_felt!(STORAGE_WRITE_LOW + STORAGE_READ_LOW), ), ( - storage_initial_values.get(&(fee_token_address, sequencer_balance_key_low)), + storage_initial_reads.get(&(fee_token_address, sequencer_balance_key_low)), stark_felt!(STORAGE_READ_LOW), ), + (storage_writes.get(&(fee_token_address, sequencer_balance_key_high)), StarkFelt::ZERO), ( - storage_write.get(&(fee_token_address, sequencer_balance_key_high)), - stark_felt!(STORAGE_WRITE_HIGH), - ), - ( - storage_initial_values.get(&(fee_token_address, sequencer_balance_key_high)), + storage_initial_reads.get(&(fee_token_address, sequencer_balance_key_high)), StarkFelt::ZERO, ), ] {