Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

Commit

Permalink
fix(transaction): remove charge fee flag from the transaction executo…
Browse files Browse the repository at this point in the history
…r execute (#1970)
  • Loading branch information
meship-starkware authored Jun 10, 2024
1 parent cefb2a2 commit 4505d16
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 39 deletions.
2 changes: 1 addition & 1 deletion crates/blockifier/src/blockifier/stateful_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ impl<S: StateReader> StatefulValidator<S> {
}

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

Expand Down
13 changes: 4 additions & 9 deletions crates/blockifier/src/blockifier/transaction_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@ 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),
);
let validate = true;
let charge_fee = true;

let tx_execution_result =
tx.execute_raw(&mut transactional_state, &self.block_context, charge_fee, validate);
Expand All @@ -114,11 +114,10 @@ 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, charge_fee) {
match self.execute(tx) {
Ok(tx_execution_info) => results.push(Ok(tx_execution_info)),
Err(TransactionExecutorError::BlockFull) => break,
Err(error) => results.push(Err(error)),
Expand All @@ -131,7 +130,6 @@ impl<S: StateReader> TransactionExecutor<S> {
pub fn execute_chunk(
&mut self,
_chunk: &[Transaction],
_charge_fee: bool,
) -> Vec<TransactionExecutorResult<TransactionExecutionInfo>> {
unimplemented!()
}
Expand Down Expand Up @@ -177,11 +175,10 @@ 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, charge_fee)
self.execute_txs_sequentially(txs)
} else {
log::debug!("Executing transactions concurrently.");
let chunk_size = self.config.concurrency_config.chunk_size;
Expand All @@ -200,7 +197,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, charge_fee);
let chunk_results = self.execute_chunk(chunk);
if chunk_results.len() < chunk.len() {
// Block is full.
results.extend(chunk_results);
Expand All @@ -218,8 +215,6 @@ impl<S: StateReader + Send + Sync> TransactionExecutor<S> {
pub fn execute_chunk(
&mut self,
chunk: &[Transaction],
// TODO(barak, 01/08/2024): Make `charge_fee` a parameter of `WorkerExecutor`.
_charge_fee: bool,
) -> Vec<TransactionExecutorResult<TransactionExecutionInfo>> {
let block_state = self.block_state.take().expect("The block state should be `Some`.");

Expand Down
37 changes: 15 additions & 22 deletions crates/blockifier/src/blockifier/transaction_executor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,14 @@ fn tx_executor_test_body<S: StateReader>(
state: CachedState<S>,
block_context: BlockContext,
tx: Transaction,
charge_fee: bool,
expected_bouncer_weights: BouncerWeights,
) {
let mut tx_executor =
TransactionExecutor::new(state, block_context, TransactionExecutorConfig::default());
// 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, charge_fee).unwrap();
let _tx_execution_info = tx_executor.execute(&tx).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 @@ -95,7 +94,6 @@ fn tx_executor_test_body<S: StateReader>(
)]
fn test_declare(
block_context: BlockContext,
#[values(true, false)] charge_fee: bool,
#[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] account_cairo_version: CairoVersion,
#[case] transaction_version: TransactionVersion,
#[case] cairo_version: CairoVersion,
Expand All @@ -115,15 +113,14 @@ fn test_declare(
},
calculate_class_info_for_testing(declared_contract.get_class()),
));
tx_executor_test_body(state, block_context, tx, charge_fee, expected_bouncer_weights);
tx_executor_test_body(state, block_context, tx, expected_bouncer_weights);
}

#[rstest]
fn test_deploy_account(
block_context: BlockContext,
#[values(TransactionVersion::ONE, TransactionVersion::THREE)] version: TransactionVersion,
#[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion,
#[values(true, false)] charge_fee: bool,
) {
let account_contract = FeatureContract::AccountWithoutValidations(cairo_version);
let state = test_state(&block_context.chain_info, BALANCE, &[(account_contract, 0)]);
Expand All @@ -142,7 +139,7 @@ fn test_deploy_account(
n_events: 0,
..Default::default()
};
tx_executor_test_body(state, block_context, tx, charge_fee, expected_bouncer_weights);
tx_executor_test_body(state, block_context, tx, expected_bouncer_weights);
}

#[rstest]
Expand Down Expand Up @@ -185,7 +182,6 @@ fn test_deploy_account(
)]
fn test_invoke(
block_context: BlockContext,
#[values(true, false)] charge_fee: bool,
#[values(TransactionVersion::ONE, TransactionVersion::THREE)] version: TransactionVersion,
#[values(CairoVersion::Cairo0, CairoVersion::Cairo1)] cairo_version: CairoVersion,
#[case] entry_point_name: &str,
Expand All @@ -207,11 +203,11 @@ fn test_invoke(
calldata,
version,
}));
tx_executor_test_body(state, block_context, tx, charge_fee, expected_bouncer_weights);
tx_executor_test_body(state, block_context, tx, expected_bouncer_weights);
}

#[rstest]
fn test_l1_handler(block_context: BlockContext, #[values(true, false)] charge_fee: bool) {
fn test_l1_handler(block_context: BlockContext) {
let test_contract = FeatureContract::TestContract(CairoVersion::Cairo1);
let state = test_state(&block_context.chain_info, BALANCE, &[(test_contract, 1)]);

Expand All @@ -225,7 +221,7 @@ fn test_l1_handler(block_context: BlockContext, #[values(true, false)] charge_fe
n_events: 0,
..Default::default()
};
tx_executor_test_body(state, block_context, tx, charge_fee, expected_bouncer_weights);
tx_executor_test_body(state, block_context, tx, expected_bouncer_weights);
}

#[rstest]
Expand Down Expand Up @@ -257,15 +253,12 @@ fn test_bouncing(#[case] initial_bouncer_weights: BouncerWeights, #[case] n_even
tx_executor.bouncer.set_accumulated_weights(initial_bouncer_weights);

tx_executor
.execute(
&Transaction::AccountTransaction(emit_n_events_tx(
n_events,
account_address,
contract_address,
nonce_manager.next(account_address),
)),
true,
)
.execute(&Transaction::AccountTransaction(emit_n_events_tx(
n_events,
account_address,
contract_address,
nonce_manager.next(account_address),
)))
.map_err(|error| panic!("{error:?}: {error}"))
.unwrap();
}
Expand Down Expand Up @@ -301,7 +294,7 @@ fn test_execute_txs_bouncing() {
.collect();

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

// Check execution results.
let expected_offset = 3;
Expand Down Expand Up @@ -329,12 +322,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, true);
let remaining_tx_results = tx_executor.execute_txs(remaining_txs);
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, true);
let remaining_tx_results = tx_executor.execute_txs(remaining_txs);

assert_eq!(remaining_tx_results.len(), 2);
assert!(remaining_tx_results[0].is_ok());
Expand Down
3 changes: 1 addition & 2 deletions crates/blockifier/src/test_utils/transfers_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use crate::transaction::transaction_execution::Transaction;
const N_ACCOUNTS: u16 = 10000;
const CHUNK_SIZE: usize = 10;
const RANDOMIZATION_SEED: u64 = 0;
const CHARGE_FEE: bool = false;
const TRANSACTION_VERSION: TransactionVersion = TransactionVersion(StarkFelt::ONE);

pub struct TransfersGenerator {
Expand Down Expand Up @@ -70,7 +69,7 @@ impl TransfersGenerator {
let account_tx = self.generate_transfer(sender_address, recipient_address);
chunk.push(Transaction::AccountTransaction(account_tx));
}
let results = self.executor.execute_txs(&chunk, CHARGE_FEE);
let results = self.executor.execute_txs(&chunk);
assert_eq!(results.len(), CHUNK_SIZE);
for result in results {
assert!(!result.unwrap().is_reverted());
Expand Down
7 changes: 2 additions & 5 deletions crates/native_blockifier/src/py_block_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,9 @@ impl PyBlockExecutor {
tx: &PyAny,
optional_py_class_info: Option<PyClassInfo>,
) -> NativeBlockifierResult<Py<PyBytes>> {
let charge_fee = true;
let tx_type: String = get_py_tx_type(tx).expect(PY_TX_PARSING_ERR).to_string();
let tx: Transaction = py_tx(tx, optional_py_class_info).expect(PY_TX_PARSING_ERR);
let tx_execution_info = self.tx_executor().execute(&tx, charge_fee)?;
let tx_execution_info = self.tx_executor().execute(&tx)?;
let typed_tx_execution_info = TypedTransactionExecutionInfo::from_tx_execution_info(
&self.tx_executor().block_context,
tx_execution_info,
Expand All @@ -217,8 +216,6 @@ impl PyBlockExecutor {
&mut self,
txs_with_class_infos: Vec<(&PyAny, Option<PyClassInfo>)>,
) -> Py<PyList> {
let charge_fee = true;

// Parse Py transactions.
let (tx_types, txs): (Vec<String>, Vec<Transaction>) = txs_with_class_infos
.into_iter()
Expand All @@ -231,7 +228,7 @@ impl PyBlockExecutor {
.unzip();

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

// Process results.
// TODO(Yoni, 15/5/2024): serialize concurrently.
Expand Down

0 comments on commit 4505d16

Please sign in to comment.