Skip to content

Commit

Permalink
chore(blockifier): add struct ExecutionFlags to AccTx & use instead o…
Browse files Browse the repository at this point in the history
…f transaction::ExecutionFlags
  • Loading branch information
avivg-starkware committed Dec 5, 2024
1 parent 882df1b commit 5c9934e
Show file tree
Hide file tree
Showing 22 changed files with 330 additions and 201 deletions.
6 changes: 2 additions & 4 deletions crates/blockifier/src/blockifier/transaction_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::state::cached_state::{CachedState, CommitmentStateDiff, Transactional
use crate::state::errors::StateError;
use crate::state::state_api::{StateReader, StateResult};
use crate::transaction::errors::TransactionExecutionError;
use crate::transaction::objects::{TransactionExecutionInfo, TransactionInfoCreator};
use crate::transaction::objects::TransactionExecutionInfo;
use crate::transaction::transaction_execution::Transaction;
use crate::transaction::transactions::{ExecutableTransaction, ExecutionFlags};

Expand Down Expand Up @@ -100,11 +100,9 @@ impl<S: StateReader> TransactionExecutor<S> {
let mut transactional_state = TransactionalState::create_transactional(
self.block_state.as_mut().expect(BLOCK_STATE_ACCESS_ERR),
);
let tx_charge_fee = tx.create_tx_info().enforce_fee();

// Executing a single transaction cannot be done in a concurrent mode.
let execution_flags =
ExecutionFlags { charge_fee: tx_charge_fee, validate: true, concurrency_mode: false };
let execution_flags = ExecutionFlags { concurrency_mode: false };
let tx_execution_result =
tx.execute_raw(&mut transactional_state, &self.block_context, execution_flags);
match tx_execution_result {
Expand Down
21 changes: 16 additions & 5 deletions crates/blockifier/src/blockifier/transaction_executor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use crate::test_utils::{
BALANCE,
DEFAULT_STRK_L1_GAS_PRICE,
};
use crate::transaction::account_transaction::AccountTransaction;
use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags};
use crate::transaction::errors::TransactionExecutionError;
use crate::transaction::test_utils::{
block_context,
Expand All @@ -41,6 +41,7 @@ use crate::transaction::test_utils::{
TestInitData,
};
use crate::transaction::transaction_execution::Transaction;
use crate::transaction::transactions::enforce_fee;
fn tx_executor_test_body<S: StateReader>(
state: CachedState<S>,
block_context: BlockContext,
Expand Down Expand Up @@ -131,7 +132,10 @@ fn test_declare(
},
calculate_class_info_for_testing(declared_contract.get_class()),
);
let tx = AccountTransaction { tx: declare_tx, only_query: false }.into();
let only_query = false;
let charge_fee = enforce_fee(&declare_tx, only_query);
let execution_flags = ExecutionFlags { only_query, charge_fee, ..ExecutionFlags::default() };
let tx = AccountTransaction { tx: declare_tx, execution_flags }.into();
tx_executor_test_body(state, block_context, tx, expected_bouncer_weights);
}

Expand All @@ -152,13 +156,17 @@ fn test_deploy_account(
},
&mut NonceManager::default(),
);
let tx = AccountTransaction { tx: deploy_account_tx, only_query: false }.into();
let only_query = false;
let charge_fee = enforce_fee(&deploy_account_tx, only_query);
let execution_flags = ExecutionFlags { only_query, charge_fee, ..ExecutionFlags::default() };
let tx = AccountTransaction { tx: deploy_account_tx, execution_flags }.into();
let expected_bouncer_weights = BouncerWeights {
state_diff_size: 3,
message_segment_length: 0,
n_events: 0,
..BouncerWeights::empty()
};
}
.into();
tx_executor_test_body(state, block_context, tx, expected_bouncer_weights);
}

Expand Down Expand Up @@ -223,7 +231,10 @@ fn test_invoke(
calldata,
version,
});
let tx = AccountTransaction { tx: invoke_tx, only_query: false }.into();
let only_query = false;
let charge_fee = enforce_fee(&invoke_tx, only_query);
let execution_flags = ExecutionFlags { only_query, charge_fee, ..ExecutionFlags::default() };
let tx = AccountTransaction { tx: invoke_tx, execution_flags }.into();
tx_executor_test_body(state, block_context, tx, expected_bouncer_weights);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/concurrency/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ pub fn create_fee_transfer_call_info<S: StateReader>(
) -> CallInfo {
let block_context = BlockContext::create_for_account_testing();
let mut transactional_state = TransactionalState::create_transactional(state);
let execution_flags = ExecutionFlags { charge_fee: true, validate: true, concurrency_mode };
let execution_flags = ExecutionFlags { concurrency_mode };
let execution_info =
account_tx.execute_raw(&mut transactional_state, &block_context, execution_flags).unwrap();

Expand Down
17 changes: 9 additions & 8 deletions crates/blockifier/src/concurrency/versioned_state_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ use crate::test_utils::deploy_account::deploy_account_tx;
use crate::test_utils::dict_state_reader::DictStateReader;
use crate::test_utils::initial_test_state::test_state;
use crate::test_utils::{CairoVersion, BALANCE, DEFAULT_STRK_L1_GAS_PRICE};
use crate::transaction::account_transaction::AccountTransaction;
use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags};
use crate::transaction::objects::HasRelatedFeeType;
use crate::transaction::test_utils::{default_all_resource_bounds, l1_resource_bounds};
use crate::transaction::transactions::ExecutableTransaction;
use crate::transaction::transactions::{enforce_fee, ExecutableTransaction};

#[fixture]
pub fn safe_versioned_state(
Expand Down Expand Up @@ -236,8 +236,9 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) {
},
&mut NonceManager::default(),
);
let deploy_account_tx_1 = AccountTransaction { tx, only_query: false };
let enforce_fee = deploy_account_tx_1.enforce_fee();
let enforce_fee = enforce_fee(&tx, false);
let execution_flags = ExecutionFlags { charge_fee: enforce_fee, ..ExecutionFlags::default() };
let deploy_account_tx_1 = AccountTransaction { tx, execution_flags: execution_flags.clone() };

let class_hash = grindy_account.get_class_hash();
let ctor_storage_arg = felt!(1_u8);
Expand All @@ -251,8 +252,9 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) {
let nonce_manager = &mut NonceManager::default();
let delpoy_account_tx_2 = AccountTransaction {
tx: deploy_account_tx(deploy_tx_args, nonce_manager),
only_query: false,
execution_flags,
};

let account_address = delpoy_account_tx_2.sender_address();
let tx_context = block_context.to_tx_context(&delpoy_account_tx_2);
let fee_type = tx_context.tx_info.fee_type();
Expand All @@ -269,12 +271,11 @@ fn test_run_parallel_txs(default_all_resource_bounds: ValidResourceBounds) {
// Execute transactions
thread::scope(|s| {
s.spawn(move || {
let result =
deploy_account_tx_1.execute(&mut state_1, &block_context_1, enforce_fee, true);
let result = deploy_account_tx_1.execute(&mut state_1, &block_context_1);
assert_eq!(result.is_err(), enforce_fee); // Transaction fails iff we enforced the fee charge (as the acount is not funded).
});
s.spawn(move || {
delpoy_account_tx_2.execute(&mut state_2, &block_context_2, true, true).unwrap();
delpoy_account_tx_2.execute(&mut state_2, &block_context_2).unwrap();
// Check that the constructor wrote ctor_arg to the storage.
let storage_key = get_storage_var_address("ctor_arg", &[]);
let deployed_contract_address = calculate_contract_address(
Expand Down
10 changes: 2 additions & 8 deletions crates/blockifier/src/concurrency/worker_logic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@ use crate::concurrency::TxIndex;
use crate::context::BlockContext;
use crate::state::cached_state::{ContractClassMapping, StateMaps, TransactionalState};
use crate::state::state_api::{StateReader, UpdatableState};
use crate::transaction::objects::{
TransactionExecutionInfo,
TransactionExecutionResult,
TransactionInfoCreator,
};
use crate::transaction::objects::{TransactionExecutionInfo, TransactionExecutionResult};
use crate::transaction::transaction_execution::Transaction;
use crate::transaction::transactions::{ExecutableTransaction, ExecutionFlags};

Expand Down Expand Up @@ -127,11 +123,9 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> {
fn execute_tx(&self, tx_index: TxIndex) {
let mut tx_versioned_state = self.state.pin_version(tx_index);
let tx = &self.chunk[tx_index];
let tx_charge_fee = tx.create_tx_info().enforce_fee();
let mut transactional_state =
TransactionalState::create_transactional(&mut tx_versioned_state);
let execution_flags =
ExecutionFlags { charge_fee: tx_charge_fee, validate: true, concurrency_mode: true };
let execution_flags = ExecutionFlags { concurrency_mode: true };
let execution_result =
tx.execute_raw(&mut transactional_state, self.block_context, execution_flags);

Expand Down
5 changes: 3 additions & 2 deletions crates/blockifier/src/concurrency/worker_logic_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use crate::test_utils::{
BALANCE,
TEST_ERC20_CONTRACT_ADDRESS2,
};
use crate::transaction::account_transaction::AccountTransaction;
use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags};
use crate::transaction::objects::HasRelatedFeeType;
use crate::transaction::test_utils::{
account_invoke_tx,
Expand Down Expand Up @@ -561,7 +561,8 @@ fn test_deploy_before_declare(
},
test_class_info.clone(),
);
let declare_tx = AccountTransaction { tx, only_query: false };
let execution_flags = ExecutionFlags::default();
let declare_tx = AccountTransaction { tx, execution_flags };

// Deploy test contract.
let invoke_tx = account_invoke_tx(invoke_tx_args! {
Expand Down
14 changes: 9 additions & 5 deletions crates/blockifier/src/execution/stack_trace_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ use crate::execution::syscalls::hint_processor::ENTRYPOINT_FAILED_ERROR;
use crate::test_utils::contracts::FeatureContract;
use crate::test_utils::initial_test_state::{fund_account, test_state};
use crate::test_utils::{create_calldata, CairoVersion, BALANCE};
use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags};
use crate::transaction::test_utils::{
account_invoke_tx,
block_context,
Expand Down Expand Up @@ -608,6 +609,11 @@ fn test_validate_trace(
}
}

// TODO(AvivG): Change this fixup to not create account_tx twice w wrong charge_fee.
let execution_flags =
ExecutionFlags { charge_fee: account_tx.enforce_fee(), ..ExecutionFlags::default() };
let account_tx = AccountTransaction { tx: account_tx.tx, execution_flags };

let contract_address = *sender_address.0.key();

let expected_error = match cairo_version {
Expand Down Expand Up @@ -639,8 +645,7 @@ Error in contract (contract address: {contract_address:#064x}, class hash: {:#06
// Clean pc locations from the trace.
let re = Regex::new(r"pc=0:[0-9]+").unwrap();
let cleaned_expected_error = &re.replace_all(&expected_error, "pc=0:*");
let charge_fee = account_tx.enforce_fee();
let actual_error = account_tx.execute(state, block_context, charge_fee, true).unwrap_err();
let actual_error = account_tx.execute(state, block_context).unwrap_err();
let actual_error_str = actual_error.to_string();
let cleaned_actual_error = &re.replace_all(&actual_error_str, "pc=0:*");
// Compare actual trace to the expected trace (sans pc locations).
Expand Down Expand Up @@ -713,7 +718,7 @@ Error in contract (contract address: {expected_address:#064x}, class hash: {:#06
};

// Compare expected and actual error.
let error = deploy_account_tx.execute(state, &block_context, true, true).unwrap_err();
let error = deploy_account_tx.execute(state, &block_context).unwrap_err();
assert_eq!(error.to_string(), expected_error);
}

Expand Down Expand Up @@ -854,8 +859,7 @@ Error in contract (contract address: {expected_address:#064x}, class hash: {:#06
};

// Compare expected and actual error.
let error =
invoke_deploy_tx.execute(state, &block_context, true, true).unwrap().revert_error.unwrap();
let error = invoke_deploy_tx.execute(state, &block_context).unwrap().revert_error.unwrap();
assert_eq!(error.to_string(), expected_error);
}

Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/fee/receipt_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ fn test_calculate_tx_gas_usage(
let calldata_length = account_tx.calldata_length();
let signature_length = account_tx.signature_length();
let fee_token_address = chain_info.fee_token_address(&account_tx.fee_type());
let tx_execution_info = account_tx.execute(state, block_context, true, true).unwrap();
let tx_execution_info = account_tx.execute(state, block_context).unwrap();

let n_storage_updates = 1; // For the account balance update.
let n_modified_contracts = 1;
Expand Down Expand Up @@ -437,7 +437,7 @@ fn test_calculate_tx_gas_usage(

let calldata_length = account_tx.calldata_length();
let signature_length = account_tx.signature_length();
let tx_execution_info = account_tx.execute(state, block_context, true, true).unwrap();
let tx_execution_info = account_tx.execute(state, block_context).unwrap();
// For the balance update of the sender and the recipient.
let n_storage_updates = 2;
// Only the account contract modification (nonce update) excluding the fee token contract.
Expand Down
12 changes: 9 additions & 3 deletions crates/blockifier/src/test_utils/transfers_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ use crate::test_utils::dict_state_reader::DictStateReader;
use crate::test_utils::initial_test_state::test_state;
use crate::test_utils::invoke::invoke_tx;
use crate::test_utils::{CairoVersion, BALANCE, MAX_FEE};
use crate::transaction::account_transaction::AccountTransaction;
use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags};
use crate::transaction::transaction_execution::Transaction;

use crate::transaction::transactions::enforce_fee;
const N_ACCOUNTS: u16 = 10000;
const N_TXS: usize = 1000;
const RANDOMIZATION_SEED: u64 = 0;
Expand Down Expand Up @@ -145,7 +145,13 @@ impl TransfersGenerator {
self.sender_index = (self.sender_index + 1) % self.account_addresses.len();

let tx = self.generate_transfer(sender_address, recipient_address);
let account_tx = AccountTransaction { tx, only_query: false };
let only_query = false;
let execution_flags = ExecutionFlags {
charge_fee: enforce_fee(&tx, only_query),
only_query,
..ExecutionFlags::default()
};
let account_tx = AccountTransaction { tx, execution_flags };
txs.push(Transaction::Account(account_tx));
}
let results = self.executor.execute_txs(&txs);
Expand Down
34 changes: 25 additions & 9 deletions crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ use crate::transaction::transaction_types::TransactionType;
use crate::transaction::transactions::{
Executable,
ExecutableTransaction,
ExecutionFlags,
ExecutionFlags as TransactionExecutionFlags,
ValidatableTransaction,
};

Expand All @@ -77,11 +77,24 @@ mod flavors_test;
#[path = "post_execution_test.rs"]
mod post_execution_test;

#[derive(Clone, Debug, derive_more::From)]
pub struct ExecutionFlags {
pub only_query: bool,
pub charge_fee: bool,
pub validate: bool,
}

impl Default for ExecutionFlags {
fn default() -> Self {
Self { only_query: false, charge_fee: true, validate: true }
}
}

/// Represents a paid Starknet transaction.
#[derive(Clone, Debug, derive_more::From)]
pub struct AccountTransaction {
pub tx: Transaction,
pub only_query: bool,
pub execution_flags: ExecutionFlags,
}
// TODO(AvivG): create additional macro that returns a reference.
macro_rules! implement_tx_getter_calls {
Expand Down Expand Up @@ -716,7 +729,7 @@ impl<U: UpdatableState> ExecutableTransaction<U> for AccountTransaction {
&self,
state: &mut TransactionalState<'_, U>,
block_context: &BlockContext,
execution_flags: ExecutionFlags,
execution_flags_: TransactionExecutionFlags,
) -> TransactionExecutionResult<TransactionExecutionInfo> {
let tx_context = Arc::new(block_context.to_tx_context(self));
self.verify_tx_version(tx_context.tx_info.version())?;
Expand All @@ -726,7 +739,8 @@ impl<U: UpdatableState> ExecutableTransaction<U> for AccountTransaction {
self.perform_pre_validation_stage(
state,
&tx_context,
execution_flags.charge_fee,
// TODO (AvivG): access execution_flags within func instead of passing them as args.
self.execution_flags.charge_fee,
strict_nonce_check,
)?;

Expand All @@ -747,15 +761,17 @@ impl<U: UpdatableState> ExecutableTransaction<U> for AccountTransaction {
state,
&mut remaining_gas,
tx_context.clone(),
execution_flags.validate,
execution_flags.charge_fee,
// TODO (AvivG): access execution_flags within func instead of passing them as args.
self.execution_flags.validate,
self.execution_flags.charge_fee,
)?;
let fee_transfer_call_info = Self::handle_fee(
state,
tx_context,
final_fee,
execution_flags.charge_fee,
execution_flags.concurrency_mode,
// TODO (AvivG): access execution_flags within func instead of passing them as args.
self.execution_flags.charge_fee,
execution_flags_.concurrency_mode,
)?;

let tx_execution_info = TransactionExecutionInfo {
Expand All @@ -776,7 +792,7 @@ impl<U: UpdatableState> ExecutableTransaction<U> for AccountTransaction {

impl TransactionInfoCreator for AccountTransaction {
fn create_tx_info(&self) -> TransactionInfo {
self.tx.create_tx_info(self.only_query)
self.tx.create_tx_info(self.execution_flags.only_query)
}
}

Expand Down
Loading

0 comments on commit 5c9934e

Please sign in to comment.