test(concurrency): write commit transaction test (#1893)

crates/blockifier/src/concurrency/fee_utils.rs

@@ -8,7 +8,7 @@ mod test;
 // We read account balance (sender), and sequencer balance (recipient). The balance is of type
 // `Uint256`, consist of two felts (lsb, msb). Hence, storage read values =
 // [account_balance, 0, sequencer_balance, 0]
-const STORAGE_READ_SEQUENCER_BALANCE_INDICES: (usize, usize) = (2, 3);
+pub(crate) const STORAGE_READ_SEQUENCER_BALANCE_INDICES: (usize, usize) = (2, 3);
 
 // Completes the fee transfer execution by fixing the call info to have the correct sequencer
 // balance. In concurrency mode, the fee transfer is executed with a false (constant) sequencer

crates/blockifier/src/concurrency/worker_logic_test.rs

@@ -11,9 +11,11 @@ use super::WorkerExecutor;
 use crate::abi::abi_utils::get_fee_token_var_address;
 use crate::abi::sierra_types::next_storage_key;
 use crate::bouncer::Bouncer;
+use crate::concurrency::fee_utils::STORAGE_READ_SEQUENCER_BALANCE_INDICES;
 use crate::concurrency::scheduler::{Task, TransactionStatus};
 use crate::concurrency::test_utils::safe_versioned_state_for_testing;
-use crate::concurrency::worker_logic::add_fee_to_sequencer_balance;
+use crate::concurrency::versioned_state::ThreadSafeVersionedState;
+use crate::concurrency::worker_logic::{add_fee_to_sequencer_balance, lock_mutex_in_array};
 use crate::context::{BlockContext, TransactionContext};
 use crate::execution::execution_utils::{felt_to_stark_felt, stark_felt_to_felt};
 use crate::fee::fee_utils::get_sequencer_balance_keys;
@@ -35,7 +37,7 @@ use crate::transaction::test_utils::{
 use crate::transaction::transaction_execution::Transaction;
 use crate::{declare_tx_args, invoke_tx_args, nonce, storage_key};
 
-fn _trivial_calldata_invoke_tx(
+fn trivial_calldata_invoke_tx(
     account_address: ContractAddress,
     test_contract_address: ContractAddress,
     nonce: Nonce,
@@ -50,19 +52,19 @@ fn _trivial_calldata_invoke_tx(
 }
 
 /// Checks that the sequencer balance was updated as expected in the state.
-fn _verify_sequencer_balance_update<S: StateReader>(
-    executor: &WorkerExecutor<'_, S>,
+fn verify_sequencer_balance_update<S: StateReader>(
+    state: &ThreadSafeVersionedState<S>,
     tx_context: &TransactionContext,
     tx_index: usize,
     // We assume the balance is at most 2^128, so the "low" value is sufficient.
-    expected_sequencer_balance_low: StarkFelt,
+    expected_sequencer_balance_low: u128,
 ) {
     let TransactionContext { block_context, tx_info } = tx_context;
-    let tx_version_state = executor.state.pin_version(tx_index);
+    let tx_version_state = state.pin_version(tx_index);
     let (sequencer_balance_key_low, sequencer_balance_key_high) =
         get_sequencer_balance_keys(block_context);
     for (expected_balance, storage_key) in [
-        (expected_sequencer_balance_low, sequencer_balance_key_low),
+        (stark_felt!(expected_sequencer_balance_low), sequencer_balance_key_low),
         (StarkFelt::ZERO, sequencer_balance_key_high),
     ] {
         let actual_balance = tx_version_state
@@ -75,6 +77,112 @@ fn _verify_sequencer_balance_update<S: StateReader>(
     }
 }
 
+#[rstest]
+pub fn test_commit_tx() {
+    let block_context = BlockContext::create_for_account_testing_with_concurrency_mode(true);
+    let account = FeatureContract::AccountWithoutValidations(CairoVersion::Cairo1);
+    let test_contract = FeatureContract::TestContract(CairoVersion::Cairo0);
+    let mut expected_sequencer_balance_low = 0_u128;
+    let mut nonce_manager = NonceManager::default();
+    let account_address = account.get_instance_address(0);
+    let test_contract_address = test_contract.get_instance_address(0);
+    let first_nonce = nonce_manager.next(account_address);
+    let second_nonce = nonce_manager.next(account_address);
+
+    // Create transactions.
+    let txs = [
+        trivial_calldata_invoke_tx(account_address, test_contract_address, first_nonce),
+        trivial_calldata_invoke_tx(account_address, test_contract_address, second_nonce),
+        trivial_calldata_invoke_tx(account_address, test_contract_address, second_nonce),
+        // Invalid nonce.
+        trivial_calldata_invoke_tx(account_address, test_contract_address, nonce!(10_u8)),
+    ]
+    .into_iter()
+    .map(Transaction::AccountTransaction)
+    .collect::<Vec<Transaction>>();
+    let mut bouncer = Bouncer::new(block_context.bouncer_config.clone());
+    let cached_state =
+        test_state(&block_context.chain_info, BALANCE, &[(account, 1), (test_contract, 1)]);
+    let versioned_state = safe_versioned_state_for_testing(cached_state);
+    let executor =
+        WorkerExecutor::new(versioned_state, &txs, &block_context, Mutex::new(&mut bouncer));
+
+    // Execute transactions.
+    // Simulate a concurrent run by executing tx1 before tx0.
+    // tx1 should fail execution since its nonce equals 1, and it is being executed before tx0,
+    // whose nonce equals 0.
+    // tx0 should pass execution.
+    // tx2 should pass execution since its nonce equals 1, so executing it after tx0 should
+    // succeed.
+    // tx3 should fail execution regardless of execution order since its nonce
+    // equals 10, where there are only four transactions.
+    for &(execute_idx, should_fail_execution) in
+        [(1, true), (0, false), (2, false), (3, true)].iter()
+    {
+        executor.execute_tx(execute_idx);
+        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);
+        if !should_fail_execution {
+            assert!(!result.as_ref().unwrap().is_reverted());
+        }
+    }
+
+    // Commit all transactions in sequential order.
+    // * tx0 should pass revalidation, fix the sequencer balance, fix the call info (fee transfer)
+    //   and commit.
+    // * tx1 should fail revalidation (it read the nonce before tx0 incremented it). It should pass
+    //   re-execution (since tx0 incremented the nonce), fix the sequencer balance, fix the call
+    //   info (fee transfer) and commit.
+    // * tx2 should fail revalidation (it read the nonce before tx1 re-executed and incremented it).
+    //   It should fail re-execution because it has the same nonce as tx1.
+    // * tx3 should pass revalidation and commit.
+    for &(commit_idx, should_fail_execution) in
+        [(0, false), (1, false), (2, true), (3, true)].iter()
+    {
+        executor.commit_tx(commit_idx);
+        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;
+        assert_eq!(execution_result.is_err(), should_fail_execution);
+        // Extract the actual fee. If the transaction fails, no fee should be charged.
+        let actual_fee = if should_fail_execution {
+            0
+        } else {
+            execution_result.as_ref().unwrap().transaction_receipt.fee.0
+        };
+        if !should_fail_execution {
+            assert!(!execution_result.as_ref().unwrap().is_reverted());
+            // Check that the call info was fixed.
+            for (expected_sequencer_storage_read, read_storage_index) in [
+                (expected_sequencer_balance_low, STORAGE_READ_SEQUENCER_BALANCE_INDICES.0),
+                (expected_sequencer_balance_high, STORAGE_READ_SEQUENCER_BALANCE_INDICES.1),
+            ] {
+                let actual_sequencer_storage_read = execution_result
+                    .as_ref()
+                    .unwrap()
+                    .fee_transfer_call_info
+                    .as_ref()
+                    .unwrap()
+                    .storage_read_values[read_storage_index];
+                assert_eq!(
+                    stark_felt!(expected_sequencer_storage_read),
+                    actual_sequencer_storage_read,
+                );
+            }
+        }
+        let tx_context = executor.block_context.to_tx_context(&txs[commit_idx]);
+        expected_sequencer_balance_low += actual_fee;
+        // Check that the sequencer balance was updated correctly in the state.
+        verify_sequencer_balance_update(
+            &executor.state,
+            &tx_context,
+            commit_idx,
+            expected_sequencer_balance_low,
+        );
+    }
+}
+
 #[test]
 fn test_worker_execute() {
     // Settings.