Skip to content

Commit

Permalink
chore(blockifier): ues account_transaction::ExecutionFlags instead of…
Browse files Browse the repository at this point in the history
… transaction::ExecutionFlags
  • Loading branch information
avivg-starkware committed Dec 3, 2024
1 parent 3106183 commit 3e3f21e
Show file tree
Hide file tree
Showing 17 changed files with 164 additions and 77 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
18 changes: 14 additions & 4 deletions crates/blockifier/src/blockifier/transaction_executor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,9 @@ fn test_declare(
},
calculate_class_info_for_testing(declared_contract.get_class()),
);
let execution_flags = ExecutionFlags::default();
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 @@ -153,13 +156,17 @@ fn test_deploy_account(
},
&mut NonceManager::default(),
);
let tx = AccountTransaction { tx: deploy_account_tx, execution_flags: ExecutionFlags::default() }.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 @@ -224,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
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
6 changes: 6 additions & 0 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
12 changes: 6 additions & 6 deletions crates/blockifier/src/transaction/account_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ impl<U: UpdatableState> ExecutableTransaction<U> for AccountTransaction {
&self,
state: &mut TransactionalState<'_, U>,
block_context: &BlockContext,
execution_flags: TransactionExecutionFlags,
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 @@ -731,7 +731,7 @@ impl<U: UpdatableState> ExecutableTransaction<U> for AccountTransaction {
self.perform_pre_validation_stage(
state,
&tx_context,
execution_flags.charge_fee,
self.execution_flags.charge_fee,
strict_nonce_check,
)?;

Expand All @@ -752,15 +752,15 @@ impl<U: UpdatableState> ExecutableTransaction<U> for AccountTransaction {
state,
&mut remaining_gas,
tx_context.clone(),
execution_flags.validate,
execution_flags.charge_fee,
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 Down
12 changes: 5 additions & 7 deletions crates/blockifier/src/transaction/account_transactions_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1372,8 +1372,7 @@ fn test_count_actual_storage_changes(
nonce: nonce_manager.next(account_address),
};
let account_tx = account_invoke_tx(invoke_args.clone());
let execution_flags =
ExecutionFlags { charge_fee: true, validate: true, concurrency_mode: false };
let execution_flags = ExecutionFlags { concurrency_mode: false };
let execution_info =
account_tx.execute_raw(&mut state, &block_context, execution_flags).unwrap();

Expand Down Expand Up @@ -1543,8 +1542,7 @@ fn test_concurrency_execute_fee_transfer(
// Case 1: The transaction did not read form/ write to the sequenser balance before executing
// fee transfer.
let mut transactional_state = TransactionalState::create_transactional(state);
let execution_flags =
ExecutionFlags { charge_fee: true, validate: true, concurrency_mode: true };
let execution_flags = ExecutionFlags { concurrency_mode: true };
let result =
account_tx.execute_raw(&mut transactional_state, &block_context, execution_flags).unwrap();
assert!(!result.is_reverted());
Expand Down Expand Up @@ -1639,8 +1637,7 @@ fn test_concurrent_fee_transfer_when_sender_is_sequencer(
let fee_token_address = block_context.chain_info.fee_token_address(fee_type);

let mut transactional_state = TransactionalState::create_transactional(state);
let execution_flags =
ExecutionFlags { charge_fee: true, validate: true, concurrency_mode: true };
let execution_flags = ExecutionFlags { concurrency_mode: true };
let result =
account_tx.execute_raw(&mut transactional_state, &block_context, execution_flags).unwrap();
assert!(!result.is_reverted());
Expand Down Expand Up @@ -1771,7 +1768,8 @@ fn test_revert_in_execute(
resource_bounds: default_all_resource_bounds,
..tx_args
});
let account_tx = AccountTransaction { tx, only_query: false };
let execution_flags = AccountExecutionFlags { validate, ..AccountExecutionFlags::default() };
let account_tx = AccountTransaction { tx, execution_flags };
let tx_execution_info = account_tx.execute(state, &block_context, true, validate).unwrap();

assert!(tx_execution_info.is_reverted());
Expand Down
63 changes: 45 additions & 18 deletions crates/blockifier/src/transaction/execution_flavors_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use crate::test_utils::{
DEFAULT_STRK_L1_GAS_PRICE,
MAX_FEE,
};
use crate::transaction::account_transaction::AccountTransaction;
use crate::transaction::account_transaction::{AccountTransaction, ExecutionFlags};
use crate::transaction::errors::{
TransactionExecutionError,
TransactionFeeError,
Expand Down Expand Up @@ -226,7 +226,8 @@ fn test_invalid_nonce_pre_validate(
let invalid_nonce = nonce!(7_u8);
let account_nonce = state.get_nonce_at(account_address).unwrap();
let tx = invoke_tx(invoke_tx_args! {nonce: invalid_nonce, ..pre_validation_base_args});
let account_tx = AccountTransaction { tx, only_query };
let execution_flags = ExecutionFlags { only_query, charge_fee, validate };
let account_tx = AccountTransaction { tx, execution_flags };
let result = account_tx.execute(&mut state, &block_context, charge_fee, validate);
assert_matches!(
result.unwrap_err(),
Expand Down Expand Up @@ -268,6 +269,7 @@ fn test_simulate_validate_pre_validate_with_charge_fee(
max_fee: Fee(10),
resource_bounds: l1_resource_bounds(10_u8.into(), 10_u8.into()),
nonce: nonce_manager.next(account_address),

..pre_validation_base_args.clone()
})
.execute(&mut state, &block_context, charge_fee, validate)
Expand Down Expand Up @@ -307,7 +309,10 @@ fn test_simulate_validate_pre_validate_with_charge_fee(

..pre_validation_base_args.clone()
});
let account_tx = AccountTransaction { tx, only_query };
let account_tx = AccountTransaction {
tx,
execution_flags: ExecutionFlags { only_query, charge_fee, validate },
};
let result = account_tx.execute(&mut state, &block_context, charge_fee, validate);

nonce_manager.rollback(account_address);
Expand Down Expand Up @@ -340,7 +345,10 @@ fn test_simulate_validate_pre_validate_with_charge_fee(

..pre_validation_base_args
});
let account_tx = AccountTransaction { tx, only_query };
let account_tx = AccountTransaction {
tx,
execution_flags: ExecutionFlags { only_query, charge_fee, validate },
};
let err = account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap_err();

nonce_manager.rollback(account_address);
Expand Down Expand Up @@ -378,10 +386,12 @@ fn test_simulate_validate_pre_validate_not_charge_fee(
nonce: nonce_manager.next(account_address),
..pre_validation_base_args.clone()
});
let account_tx = AccountTransaction { tx, only_query };
let account_tx = AccountTransaction {
tx,
execution_flags: ExecutionFlags { only_query, charge_fee, validate: false },
};
let tx_execution_info =
account_tx.execute(&mut state, &block_context, charge_fee, false).unwrap();

let base_gas = calculate_actual_gas(&tx_execution_info, &block_context, false);
assert!(
base_gas
Expand All @@ -402,10 +412,12 @@ fn test_simulate_validate_pre_validate_not_charge_fee(

..pre_validation_base_args.clone()
});
let account_tx = AccountTransaction { tx, only_query };
let account_tx = AccountTransaction {
tx,
execution_flags: ExecutionFlags { only_query, charge_fee, validate },
};
let tx_execution_info =
account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap();

check_gas_and_fee(
&block_context,
&tx_execution_info,
Expand Down Expand Up @@ -470,7 +482,10 @@ fn execute_fail_validation(
version,
nonce: nonce_manager.next(faulty_account_address),
});
let account_tx = AccountTransaction { tx, only_query };
let account_tx = AccountTransaction {
tx,
execution_flags: ExecutionFlags { only_query, charge_fee, validate },
};
account_tx.execute(&mut falliable_state, &block_context, charge_fee, validate)
}

Expand Down Expand Up @@ -593,7 +608,10 @@ fn test_simulate_validate_charge_fee_mid_execution(
only_query,
..execution_base_args.clone()
});
let account_tx = AccountTransaction { tx, only_query };
let account_tx = AccountTransaction {
tx,
execution_flags: ExecutionFlags { only_query, charge_fee, validate },
};
let tx_execution_info =
account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap();
let base_gas = calculate_actual_gas(&tx_execution_info, &block_context, validate);
Expand Down Expand Up @@ -644,10 +662,12 @@ fn test_simulate_validate_charge_fee_mid_execution(

..execution_base_args.clone()
});
let account_tx = AccountTransaction { tx, only_query };
let account_tx = AccountTransaction {
tx,
execution_flags: ExecutionFlags { only_query, charge_fee, validate },
};
let tx_execution_info =
account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap();

assert_eq!(tx_execution_info.is_reverted(), charge_fee);
if charge_fee {
assert!(
Expand Down Expand Up @@ -692,17 +712,20 @@ fn test_simulate_validate_charge_fee_mid_execution(
GasVector::from_l1_gas(block_limit_gas),
&fee_type,
);

let tx = invoke_tx(invoke_tx_args! {
max_fee: huge_fee,
resource_bounds: l1_resource_bounds(huge_gas_limit, gas_price.into()),
calldata: recurse_calldata(test_contract_address, false, 10000),
nonce: nonce_manager.next(account_address),
..execution_base_args
});
let account_tx = AccountTransaction { tx, only_query };
let account_tx = AccountTransaction {
tx,
execution_flags: ExecutionFlags { only_query, charge_fee, validate },
};
let tx_execution_info =
account_tx.execute(&mut state, &low_step_block_context, charge_fee, validate).unwrap();

assert!(
tx_execution_info.revert_error.clone().unwrap().to_string().contains("no remaining steps")
);
Expand Down Expand Up @@ -788,10 +811,12 @@ fn test_simulate_validate_charge_fee_post_execution(
version,
only_query,
});
let account_tx = AccountTransaction { tx, only_query };
let account_tx = AccountTransaction {
tx,
execution_flags: ExecutionFlags { only_query, charge_fee, validate },
};
let tx_execution_info =
account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap();

assert_eq!(tx_execution_info.is_reverted(), charge_fee);
if charge_fee {
let expected_error_prefix =
Expand Down Expand Up @@ -851,10 +876,12 @@ fn test_simulate_validate_charge_fee_post_execution(
only_query,

});
let account_tx = AccountTransaction { tx, only_query };
let account_tx = AccountTransaction {
tx,
execution_flags: ExecutionFlags { only_query, charge_fee, validate },
};
let tx_execution_info =
account_tx.execute(&mut state, &block_context, charge_fee, validate).unwrap();

assert_eq!(tx_execution_info.is_reverted(), charge_fee);

if charge_fee {
Expand Down
Loading

0 comments on commit 3e3f21e

Please sign in to comment.