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

chore(blockifier): add struct ExecutionFlags to AccTx & use instead of transaction::ExecutionFlags #2429

Merged
Merged
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: 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
33 changes: 24 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,16 @@ 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,
self.execution_flags.charge_fee,
execution_flags_.concurrency_mode,
)?;

let tx_execution_info = TransactionExecutionInfo {
Expand All @@ -776,7 +791,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
Loading