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

fix(transaction): remove charge fee flag from the transaction executo… #1970

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
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
Loading