fix(concurrency): in versioned storage - read the writes of the curre…

…nt transaction (#1944)

crates/blockifier/src/concurrency/versioned_state.rs

@@ -93,7 +93,8 @@ impl<S: StateReader> VersionedState<S> {
         if tx_index == 0 {
             return true;
         }
-
+        // Ignore values written by the current transaction.
+        let tx_index = tx_index - 1;
         for (&(contract_address, storage_key), expected_value) in &reads.storage {
             let value =
                 self.storage.read(tx_index, (contract_address, storage_key)).expect(READ_ERR);

crates/blockifier/src/concurrency/versioned_state_test.rs

@@ -174,10 +174,10 @@ fn test_versioned_state_proxy() {
     assert!(
         versioned_state_proxys[4].state().declared_contracts.read(4, another_class_hash).unwrap()
     );
-    // Ignore the writes in the current transaction.
+    // Include the writes in the current transaction.
     assert_eq!(
         versioned_state_proxys[10].get_class_hash_at(contract_address).unwrap(),
-        class_hash_v7
+        class_hash_v10
     );
     assert_eq!(
         versioned_state_proxys[2].get_compiled_class_hash(class_hash).unwrap(),

crates/blockifier/src/concurrency/versioned_storage.rs

@@ -40,9 +40,8 @@ where
     V: Clone + Debug,
 {
     pub fn read(&self, tx_index: TxIndex, key: K) -> Option<V> {
-        // Ignore the writes in the current transaction (may contain an `ESTIMATE` value). Reading
-        // the value written in this transaction should be handled by the state.
-        let value = self.writes.get(&key).and_then(|cell| cell.range(..tx_index).next_back());
+        // TODO: Ignore `ESTIMATE` values (when added).
+        let value = self.writes.get(&key).and_then(|cell| cell.range(..=tx_index).next_back());
         value.map(|(_, value)| value).or_else(|| self.cached_initial_values.get(&key)).cloned()
     }
 

crates/blockifier/src/concurrency/versioned_storage_test.rs

@@ -32,8 +32,8 @@ fn test_versioned_storage() {
     // Read from the past.
     storage.write(2, 10, 78);
     assert_eq!(storage.read(1, 10).unwrap(), 31);
-    // Ignore the value written by the current transaction.
-    assert_eq!(storage.read(2, 10).unwrap(), 31);
+    // Include the value written by the current transaction.
+    assert_eq!(storage.read(2, 10).unwrap(), 78);
 
     // Read uninitialized cell.
     assert!(storage.read(1, 100).is_none());

crates/blockifier/src/concurrency/worker_logic.rs

@@ -258,9 +258,7 @@ impl<'a, S: StateReader> WorkerExecutor<'a, S> {
                 return true;
             }
 
-            let mut next_tx_versioned_state = self.state.pin_version(tx_index + 1);
-            let (sequencer_balance_value_low, sequencer_balance_value_high) =
-                next_tx_versioned_state
+            let (sequencer_balance_value_low, sequencer_balance_value_high) = tx_versioned_state
                     .get_fee_token_balance(
                         tx_context.block_context.block_info.sequencer_address,
                         tx_context.fee_token_address(),

crates/blockifier/src/concurrency/worker_logic_test.rs

@@ -116,7 +116,7 @@ fn test_worker_execute() {
     // Read a write made by the transaction.
     assert_eq!(
         safe_versioned_state
-            .pin_version(tx_index + 1)
+            .pin_version(tx_index)
             .get_storage_at(test_contract_address, storage_key)
             .unwrap(),
         storage_value
@@ -176,7 +176,7 @@ fn test_worker_execute() {
     worker_executor.execute(tx_index);
     // No write was made by the transaction.
     assert_eq!(
-        safe_versioned_state.pin_version(tx_index + 1).get_nonce_at(account_address).unwrap(),
+        safe_versioned_state.pin_version(tx_index).get_nonce_at(account_address).unwrap(),
         nonce!(1_u8)
     );
     let execution_output = worker_executor.execution_outputs[tx_index].lock().unwrap();
@@ -195,7 +195,7 @@ fn test_worker_execute() {
     worker_executor.execute(tx_index);
     // Read a write made by the transaction.
     assert_eq!(
-        safe_versioned_state.pin_version(tx_index + 1).get_nonce_at(account_address).unwrap(),
+        safe_versioned_state.pin_version(tx_index).get_nonce_at(account_address).unwrap(),
         nonce!(2_u8)
     );
     let execution_output = worker_executor.execution_outputs[tx_index].lock().unwrap();
@@ -289,7 +289,7 @@ fn test_worker_validate() {
     // Verify writes exist in state.
     assert_eq!(
         safe_versioned_state
-            .pin_version(tx_index + 1)
+            .pin_version(tx_index)
             .get_storage_at(test_contract_address, storage_key)
             .unwrap(),
         storage_value0
@@ -304,7 +304,7 @@ fn test_worker_validate() {
     // Verify writes were removed.
     assert_eq!(
         safe_versioned_state
-            .pin_version(tx_index + 1)
+            .pin_version(tx_index)
             .get_storage_at(test_contract_address, storage_key)
             .unwrap(),
         storage_value0
@@ -346,14 +346,11 @@ pub fn test_add_fee_to_sequencer_balance(
         sequencer_balance_low,
         sequencer_balance_high,
     );
-    let next_tx_versioned_state = safe_versioned_state.pin_version(tx_index + 1);
-
-    let new_sequencer_balance_value_low = next_tx_versioned_state
-        .get_storage_at(fee_token_address, sequencer_balance_key_low)
-        .unwrap();
-    let new_sequencer_balance_value_high = next_tx_versioned_state
-        .get_storage_at(fee_token_address, sequencer_balance_key_high)
-        .unwrap();
+
+    let new_sequencer_balance_value_low =
+        tx_versioned_state.get_storage_at(fee_token_address, sequencer_balance_key_low).unwrap();
+    let new_sequencer_balance_value_high =
+        tx_versioned_state.get_storage_at(fee_token_address, sequencer_balance_key_high).unwrap();
     let expected_balance =
         (stark_felt_to_felt(sequencer_balance_low) + Felt252::from(actual_fee.0)).to_biguint();