Skip to content

Commit

Permalink
chore(blockifier): Add charge fee flage to execute
Browse files Browse the repository at this point in the history
  • Loading branch information
meship-starkware committed Aug 8, 2024
1 parent 78580c8 commit d93d8b5
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 55 deletions.
7 changes: 4 additions & 3 deletions crates/blockifier/src/blockifier/stateful_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,13 @@ impl<S: StateReader> StatefulValidator<S> {
&mut self,
tx: AccountTransaction,
skip_validate: bool,
charge_fee: bool,
) -> StatefulValidatorResult<()> {
// Deploy account transactions should be fully executed, since the constructor must run
// before `__validate_deploy__`. The execution already includes all necessary validations,
// so they are skipped here.
if let AccountTransaction::DeployAccount(_) = tx {
self.execute(tx)?;
self.execute(tx, charge_fee)?;
return Ok(());
}

Expand All @@ -83,8 +84,8 @@ impl<S: StateReader> StatefulValidator<S> {
Ok(())
}

fn execute(&mut self, tx: AccountTransaction) -> StatefulValidatorResult<()> {
self.tx_executor.execute(&Transaction::AccountTransaction(tx))?;
fn execute(&mut self, tx: AccountTransaction, charge_fee: bool) -> StatefulValidatorResult<()> {
self.tx_executor.execute(&Transaction::AccountTransaction(tx), charge_fee)?;
Ok(())
}

Expand Down
4 changes: 2 additions & 2 deletions crates/blockifier/src/blockifier/stateful_validator_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ fn test_transaction_validator(
// Test the stateful validator.
let mut stateful_validator = StatefulValidator::create(state, block_context);

let result = stateful_validator.perform_validations(tx, false);
let result = stateful_validator.perform_validations(tx, false, true);
assert!(result.is_ok(), "Validation failed: {:?}", result.unwrap_err());
}

Expand All @@ -92,6 +92,6 @@ fn test_transaction_validator_skip_validate() {

let mut stateful_validator = StatefulValidator::create(state, block_context);
// The transaction validations should be skipped and the function should return Ok.
let result = stateful_validator.perform_validations(tx, true);
let result = stateful_validator.perform_validations(tx, true, true);
assert_matches!(result, Ok(()));
}
14 changes: 9 additions & 5 deletions crates/blockifier/src/blockifier/transaction_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,14 @@ impl<S: StateReader> TransactionExecutor<S> {
pub fn execute(
&mut self,
tx: &Transaction,
charge_fee: bool,
) -> TransactionExecutorResult<TransactionExecutionInfo> {
let mut transactional_state = TransactionalState::create_transactional(
self.block_state.as_mut().expect(BLOCK_STATE_ACCESS_ERR),
);
// Executing a single transaction cannot be done in a concurrent mode.
let execution_flags =
ExecutionFlags { charge_fee: true, validate: true, concurrency_mode: false };
ExecutionFlags { charge_fee, validate: true, concurrency_mode: false };
let tx_execution_result =
tx.execute_raw(&mut transactional_state, &self.block_context, execution_flags);
match tx_execution_result {
Expand All @@ -118,10 +119,11 @@ impl<S: StateReader> TransactionExecutor<S> {
pub fn execute_txs_sequentially(
&mut self,
txs: &[Transaction],
charge_fee: bool,
) -> Vec<TransactionExecutorResult<TransactionExecutionInfo>> {
let mut results = Vec::new();
for tx in txs {
match self.execute(tx) {
match self.execute(tx, charge_fee) {
Ok(tx_execution_info) => results.push(Ok(tx_execution_info)),
Err(TransactionExecutorError::BlockFull) => break,
Err(error) => results.push(Err(error)),
Expand Down Expand Up @@ -179,10 +181,11 @@ impl<S: StateReader + Send + Sync> TransactionExecutor<S> {
pub fn execute_txs(
&mut self,
txs: &[Transaction],
charge_fee: bool,
) -> Vec<TransactionExecutorResult<TransactionExecutionInfo>> {
if !self.config.concurrency_config.enabled {
log::debug!("Executing transactions sequentially.");
self.execute_txs_sequentially(txs)
self.execute_txs_sequentially(txs, charge_fee)
} else {
log::debug!("Executing transactions concurrently.");
let chunk_size = self.config.concurrency_config.chunk_size;
Expand All @@ -201,7 +204,7 @@ impl<S: StateReader + Send + Sync> TransactionExecutor<S> {
);
txs.chunks(chunk_size)
.fold_while(Vec::new(), |mut results, chunk| {
let chunk_results = self.execute_chunk(chunk);
let chunk_results = self.execute_chunk(chunk, charge_fee);
if chunk_results.len() < chunk.len() {
// Block is full.
results.extend(chunk_results);
Expand All @@ -219,6 +222,7 @@ impl<S: StateReader + Send + Sync> TransactionExecutor<S> {
pub fn execute_chunk(
&mut self,
chunk: &[Transaction],
charge_fee: bool,
) -> Vec<TransactionExecutorResult<TransactionExecutionInfo>> {
use crate::concurrency::utils::AbortIfPanic;

Expand Down Expand Up @@ -246,7 +250,7 @@ impl<S: StateReader + Send + Sync> TransactionExecutor<S> {
// If a panic is not handled or the handling logic itself panics, then we abort
// the program.
if let Err(err) = catch_unwind(AssertUnwindSafe(|| {
worker_executor.run();
worker_executor.run(charge_fee);
})) {
// If the program panics here, the abort guard will exit the program.
// In this case, no panic message will be logged. Add the cargo flag
Expand Down
10 changes: 5 additions & 5 deletions crates/blockifier/src/blockifier/transaction_executor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn tx_executor_test_body<S: StateReader>(
// TODO(Arni, 30/03/2024): Consider adding a test for the transaction execution info. If A test
// should not be added, rename the test to `test_bouncer_info`.
// TODO(Arni, 30/03/2024): Test all bouncer weights.
let _tx_execution_info = tx_executor.execute(&tx).unwrap();
let _tx_execution_info = tx_executor.execute(&tx, true).unwrap();
let bouncer_weights = tx_executor.bouncer.get_accumulated_weights();
assert_eq!(bouncer_weights.state_diff_size, expected_bouncer_weights.state_diff_size);
assert_eq!(
Expand Down Expand Up @@ -269,7 +269,7 @@ fn test_bouncing(#[case] initial_bouncer_weights: BouncerWeights, #[case] n_even
account_address,
contract_address,
nonce_manager.next(account_address),
)))
)), true)
.map_err(|error| panic!("{error:?}: {error}"))
.unwrap();
}
Expand Down Expand Up @@ -305,7 +305,7 @@ fn test_execute_txs_bouncing() {
.collect();

// Run.
let results = tx_executor.execute_txs(&txs);
let results = tx_executor.execute_txs(&txs, true);

// Check execution results.
let expected_offset = 3;
Expand Down Expand Up @@ -333,12 +333,12 @@ fn test_execute_txs_bouncing() {

// Check idempotency: excess transactions should not be added.
let remaining_txs = &txs[expected_offset..];
let remaining_tx_results = tx_executor.execute_txs(remaining_txs);
let remaining_tx_results = tx_executor.execute_txs(remaining_txs, true);
assert_eq!(remaining_tx_results.len(), 0);

// Reset the bouncer and add the remaining transactions.
tx_executor.bouncer = Bouncer::new(tx_executor.block_context.bouncer_config.clone());
let remaining_tx_results = tx_executor.execute_txs(remaining_txs);
let remaining_tx_results = tx_executor.execute_txs(remaining_txs, true);

assert_eq!(remaining_tx_results.len(), 2);
assert!(remaining_tx_results[0].is_ok());
Expand Down
22 changes: 11 additions & 11 deletions crates/blockifier/src/concurrency/worker_logic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,13 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> {
}
}

pub fn run(&self) {
pub fn run(&self, charge_fee: bool) {
let mut task = Task::AskForTask;
loop {
self.commit_while_possible();
self.commit_while_possible(charge_fee);
task = match task {
Task::ExecutionTask(tx_index) => {
self.execute(tx_index);
self.execute(tx_index, charge_fee);
Task::AskForTask
}
Task::ValidationTask(tx_index) => self.validate(tx_index),
Expand All @@ -108,29 +108,29 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> {
}
}

fn commit_while_possible(&self) {
fn commit_while_possible(&self, charge_fee: bool) {
if let Some(mut transaction_committer) = self.scheduler.try_enter_commit_phase() {
while let Some(tx_index) = transaction_committer.try_commit() {
let commit_succeeded = self.commit_tx(tx_index);
let commit_succeeded = self.commit_tx(tx_index, charge_fee);
if !commit_succeeded {
transaction_committer.halt_scheduler();
}
}
}
}

fn execute(&self, tx_index: TxIndex) {
self.execute_tx(tx_index);
fn execute(&self, tx_index: TxIndex, charge_fee: bool) {
self.execute_tx(tx_index, charge_fee);
self.scheduler.finish_execution(tx_index)
}

fn execute_tx(&self, tx_index: TxIndex) {
fn execute_tx(&self, tx_index: TxIndex, charge_fee: bool) {
let mut tx_versioned_state = self.state.pin_version(tx_index);
let tx = &self.chunk[tx_index];
let mut transactional_state =
TransactionalState::create_transactional(&mut tx_versioned_state);
let execution_flags =
ExecutionFlags { charge_fee: true, validate: true, concurrency_mode: true };
ExecutionFlags { charge_fee, validate: true, concurrency_mode: true };
let execution_result =
tx.execute_raw(&mut transactional_state, self.block_context, execution_flags);

Expand Down Expand Up @@ -191,7 +191,7 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> {
/// - Else (no room), do not commit. The block should be closed without the transaction.
/// * Else (execution failed), commit the transaction without fixing the call info or
/// updating the sequencer balance.
fn commit_tx(&self, tx_index: TxIndex) -> bool {
fn commit_tx(&self, tx_index: TxIndex, charge_fee: bool) -> bool {
let execution_output = lock_mutex_in_array(&self.execution_outputs, tx_index);
let execution_output_ref = execution_output.as_ref().expect(EXECUTION_OUTPUTS_UNWRAP_ERROR);
let reads = &execution_output_ref.reads;
Expand All @@ -209,7 +209,7 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> {
// Release the execution output lock as it is acquired in execution (avoid dead-lock).
drop(execution_output);

self.execute_tx(tx_index);
self.execute_tx(tx_index, charge_fee);
self.scheduler.finish_execution_during_commit(tx_index);

let execution_output = lock_mutex_in_array(&self.execution_outputs, tx_index);
Expand Down
44 changes: 22 additions & 22 deletions crates/blockifier/src/concurrency/worker_logic_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ pub fn test_commit_tx() {
for &(execute_idx, should_fail_execution) in
[(1, true), (0, false), (2, false), (3, true)].iter()
{
executor.execute_tx(execute_idx);
executor.execute_tx(execute_idx, true);
let execution_task_outputs = lock_mutex_in_array(&executor.execution_outputs, execute_idx);
let result = &execution_task_outputs.as_ref().unwrap().result;
assert_eq!(result.is_err(), should_fail_execution);
Expand All @@ -146,7 +146,7 @@ pub fn test_commit_tx() {
for &(commit_idx, should_fail_execution) in
[(0, false), (1, false), (2, true), (3, true)].iter()
{
executor.commit_tx(commit_idx);
executor.commit_tx(commit_idx, true);
let execution_task_outputs = lock_mutex_in_array(&executor.execution_outputs, commit_idx);
let execution_result = &execution_task_outputs.as_ref().unwrap().result;
let expected_sequencer_balance_high = 0_u128;
Expand Down Expand Up @@ -220,7 +220,7 @@ fn test_commit_tx_when_sender_is_sequencer() {
let tx_versioned_state = executor.state.pin_version(tx_index);

// Execute and save the execution result.
executor.execute_tx(tx_index);
executor.execute_tx(tx_index, true);
let execution_task_outputs = lock_mutex_in_array(&executor.execution_outputs, tx_index);
let execution_result = &execution_task_outputs.as_ref().unwrap().result;
let fee_transfer_call_info =
Expand All @@ -237,7 +237,7 @@ fn test_commit_tx_when_sender_is_sequencer() {
tx_versioned_state.get_storage_at(fee_token_address, sequencer_balance_key_low).unwrap();

// Commit tx and check that the commit made no changes in the execution result or the state.
executor.commit_tx(tx_index);
executor.commit_tx(tx_index, true);
let execution_task_outputs = lock_mutex_in_array(&executor.execution_outputs, tx_index);
let commit_result = &execution_task_outputs.as_ref().unwrap().result;
let fee_transfer_call_info =
Expand Down Expand Up @@ -331,7 +331,7 @@ fn test_worker_execute(max_resource_bounds: ResourceBoundsMapping) {

// Successful execution.
let tx_index = 0;
worker_executor.execute(tx_index);
worker_executor.execute(tx_index, true);
// Read a write made by the transaction.
assert_eq!(
safe_versioned_state
Expand Down Expand Up @@ -392,7 +392,7 @@ fn test_worker_execute(max_resource_bounds: ResourceBoundsMapping) {

// Failed execution.
let tx_index = 1;
worker_executor.execute(tx_index);
worker_executor.execute(tx_index, true);
// No write was made by the transaction.
assert_eq!(
safe_versioned_state.pin_version(tx_index).get_nonce_at(account_address).unwrap(),
Expand All @@ -411,7 +411,7 @@ fn test_worker_execute(max_resource_bounds: ResourceBoundsMapping) {

// Reverted execution.
let tx_index = 2;
worker_executor.execute(tx_index);
worker_executor.execute(tx_index, true);
// Read a write made by the transaction.
assert_eq!(
safe_versioned_state.pin_version(tx_index).get_nonce_at(account_address).unwrap(),
Expand Down Expand Up @@ -491,8 +491,8 @@ fn test_worker_validate(max_resource_bounds: ResourceBoundsMapping) {
worker_executor.scheduler.next_task();

// Execute transactions in the wrong order, making the first execution invalid.
worker_executor.execute(1);
worker_executor.execute(0);
worker_executor.execute(1, true);
worker_executor.execute(0, true);

// Creates 2 active tasks.
worker_executor.scheduler.next_task();
Expand Down Expand Up @@ -600,8 +600,8 @@ fn test_deploy_before_declare(
worker_executor.scheduler.next_task();

// Execute transactions in the wrong order, making the first execution invalid.
worker_executor.execute(1);
worker_executor.execute(0);
worker_executor.execute(1, true);
worker_executor.execute(0, true);

let execution_output = worker_executor.execution_outputs[1].lock().unwrap();
let tx_execution_info = execution_output.as_ref().unwrap().result.as_ref().unwrap();
Expand All @@ -617,7 +617,7 @@ fn test_deploy_before_declare(
assert_eq!(worker_executor.validate(1), Task::ExecutionTask(1));

// Execute transaction 1 again.
worker_executor.execute(1);
worker_executor.execute(1, true);

let execution_output = worker_executor.execution_outputs[1].lock().unwrap();
assert!(!execution_output.as_ref().unwrap().result.as_ref().unwrap().is_reverted());
Expand Down Expand Up @@ -668,7 +668,7 @@ fn test_worker_commit_phase(max_resource_bounds: ResourceBoundsMapping) {
WorkerExecutor::new(safe_versioned_state, &txs, &block_context, Mutex::new(&mut bouncer));

// Try to commit before any transaction is ready.
worker_executor.commit_while_possible();
worker_executor.commit_while_possible(true);

// Verify no transaction was committed.
assert_eq!(worker_executor.scheduler.get_n_committed_txs(), 0);
Expand All @@ -681,11 +681,11 @@ fn test_worker_commit_phase(max_resource_bounds: ResourceBoundsMapping) {
assert_eq!(worker_executor.scheduler.next_task(), Task::ExecutionTask(1));

// Execute the first two transactions.
worker_executor.execute(0);
worker_executor.execute(1);
worker_executor.execute(0, true);
worker_executor.execute(1, true);

// Commit the first two transactions (only).
worker_executor.commit_while_possible();
worker_executor.commit_while_possible(true);

// Verify the commit index is now 2.
assert_eq!(worker_executor.scheduler.get_n_committed_txs(), 2);
Expand All @@ -696,10 +696,10 @@ fn test_worker_commit_phase(max_resource_bounds: ResourceBoundsMapping) {

// Create the final execution task and execute it.
assert_eq!(worker_executor.scheduler.next_task(), Task::ExecutionTask(2));
worker_executor.execute(2);
worker_executor.execute(2, true);

// Commit the third (and last) transaction.
worker_executor.commit_while_possible();
worker_executor.commit_while_possible(true);

// Verify the number of committed transactions is 3, the status of the last transaction is
// `Committed`, and the next task is `Done`.
Expand All @@ -708,7 +708,7 @@ fn test_worker_commit_phase(max_resource_bounds: ResourceBoundsMapping) {
assert_eq!(worker_executor.scheduler.next_task(), Task::Done);

// Try to commit when all transactions are already committed.
worker_executor.commit_while_possible();
worker_executor.commit_while_possible(true);
assert_eq!(worker_executor.scheduler.get_n_committed_txs(), 3);

// Make sure all transactions were executed successfully.
Expand Down Expand Up @@ -765,11 +765,11 @@ fn test_worker_commit_phase_with_halt() {
assert_eq!(worker_executor.scheduler.next_task(), Task::ExecutionTask(1));

// Execute both transactions.
worker_executor.execute(0);
worker_executor.execute(1);
worker_executor.execute(0, true);
worker_executor.execute(1, true);

// Commit both transactions.
worker_executor.commit_while_possible();
worker_executor.commit_while_possible(true);

// Verify the scheduler is halted.
assert_eq!(worker_executor.scheduler.next_task(), Task::Done);
Expand Down
2 changes: 1 addition & 1 deletion crates/blockifier/src/test_utils/transfers_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl TransfersGenerator {
let account_tx = self.generate_transfer(sender_address, recipient_address);
txs.push(Transaction::AccountTransaction(account_tx));
}
let results = self.executor.execute_txs(&txs);
let results = self.executor.execute_txs(&txs, true);
assert_eq!(results.len(), self.config.n_txs);
for result in results {
assert!(!result.unwrap().is_reverted());
Expand Down
2 changes: 1 addition & 1 deletion crates/gateway/src/stateful_transaction_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl StatefulTransactionValidatorTrait for BlockifierStatefulValidator {
account_tx: AccountTransaction,
skip_validate: bool,
) -> BlockifierStatefulValidatorResult<()> {
self.perform_validations(account_tx, skip_validate)
self.perform_validations(account_tx, skip_validate, true)
}

fn get_nonce(
Expand Down
Loading

0 comments on commit d93d8b5

Please sign in to comment.