fix: rename run_l2_tx_inner, fix panics for lib users (matter-labs#214)

* rename run_l2_tx_inner, fix panics for lib users

* add test

* undo dep changes

* cleanup comments

* inline params

* cleanup ExternalStorage

* fix test messages

src/node/eth.rs

@@ -814,7 +814,7 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> EthNamespa
     ///
     /// A `BoxFuture` containing a `jsonrpc_core::Result` that resolves to an array of logs, block hashes, or transaction hashes,
     /// depending on the filter type, which occurred since last poll.
-    /// * Filters created with `eth_newFilter` return [Log] objects.
+    /// * Filters created with `eth_newFilter` return [zksync_types::api::Log] objects.
     /// * Filters created with `eth_newBlockFilter` return block hashes.
     /// * Filters created with `eth_newPendingTransactionFilter` return transaction hashes.
     fn get_filter_changes(&self, id: U256) -> RpcResult<FilterChanges> {

src/node/in_memory.rs

@@ -1253,7 +1253,26 @@ impl<S: ForkSource + std::fmt::Debug + Clone> InMemoryNode<S> {
     }
 
     /// Executes the given L2 transaction and returns all the VM logs.
-    pub fn run_l2_tx_inner(
+    ///
+    /// **NOTE**
+    ///
+    /// This function must only rely on data populated initially via [ForkDetails]:
+    ///     * [InMemoryNodeInner::current_timestamp]
+    ///     * [InMemoryNodeInner::current_batch]
+    ///     * [InMemoryNodeInner::current_miniblock]
+    ///     * [InMemoryNodeInner::current_miniblock_hash]
+    ///     * [InMemoryNodeInner::l1_gas_price]
+    ///
+    /// And must _NEVER_ rely on data updated in [InMemoryNodeInner] during previous runs:
+    /// (if used, they must never panic and/or have meaningful defaults)
+    ///     * [InMemoryNodeInner::block_hashes]
+    ///     * [InMemoryNodeInner::blocks]
+    ///     * [InMemoryNodeInner::tx_results]
+    ///
+    /// This is because external users of the library may call this function to perform an isolated
+    /// VM operation with an external storage and get the results back.
+    /// So any data populated in [Self::run_l2_tx] will not be available for the next invocation.
+    pub fn run_l2_tx_raw(
         &self,
         l2_tx: L2Tx,
         execution_mode: TxExecutionMode,
@@ -1420,17 +1439,14 @@ impl<S: ForkSource + std::fmt::Debug + Clone> InMemoryNode<S> {
         let hash = compute_hash(block_ctx.miniblock, l2_tx.hash());
 
         let mut transaction = zksync_types::api::Transaction::from(l2_tx);
-        let block_hash = inner
-            .block_hashes
-            .get(&inner.current_miniblock)
-            .ok_or(format!(
-                "Block hash not found for block: {}",
-                inner.current_miniblock
-            ))?;
-        transaction.block_hash = Some(*block_hash);
+        transaction.block_hash = Some(inner.current_miniblock_hash);
         transaction.block_number = Some(U64::from(inner.current_miniblock));
 
-        let parent_block_hash = *inner.block_hashes.get(&(block_ctx.miniblock - 1)).unwrap();
+        let parent_block_hash = inner
+            .block_hashes
+            .get(&(block_ctx.miniblock - 1))
+            .cloned()
+            .unwrap_or_default();
 
         let block = Block {
             hash,
@@ -1491,7 +1507,7 @@ impl<S: ForkSource + std::fmt::Debug + Clone> InMemoryNode<S> {
         }
 
         let (keys, result, call_traces, block, bytecodes, block_ctx) =
-            self.run_l2_tx_inner(l2_tx.clone(), execution_mode)?;
+            self.run_l2_tx_raw(l2_tx.clone(), execution_mode)?;
 
         if let ExecutionResult::Halt { reason } = result.result {
             // Halt means that something went really bad with the transaction execution (in most cases invalid signature,
@@ -1780,4 +1796,39 @@ mod tests {
         assert_eq!(first_block.parent_hash, compute_hash(123, H256::zero()));
         assert_eq!(second_block.parent_hash, first_block.hash);
     }
+
+    #[tokio::test]
+    async fn test_run_l2_tx_raw_does_not_panic_on_external_storage_call() {
+        // Perform a transaction to get storage to an intermediate state
+        let node = InMemoryNode::<HttpForkSource>::default();
+        let tx = testing::TransactionBuilder::new().build();
+        node.set_rich_account(tx.common_data.initiator_address);
+        node.run_l2_tx(tx, TxExecutionMode::VerifyExecute).unwrap();
+        let external_storage = node.inner.read().unwrap().fork_storage.clone();
+
+        // Execute next transaction using a fresh in-memory node and the external fork storage
+        let mock_db = testing::ExternalStorage {
+            raw_storage: external_storage.inner.read().unwrap().raw_storage.clone(),
+        };
+        let node = InMemoryNode::new(
+            Some(ForkDetails {
+                fork_source: &mock_db,
+                l1_block: L1BatchNumber(1),
+                l2_block: Block::default(),
+                l2_miniblock: 2,
+                l2_miniblock_hash: Default::default(),
+                block_timestamp: 1002,
+                overwrite_chain_id: None,
+                l1_gas_price: 1000,
+            }),
+            None,
+            Default::default(),
+        );
+
+        node.run_l2_tx_raw(
+            testing::TransactionBuilder::new().build(),
+            TxExecutionMode::VerifyExecute,
+        )
+        .expect("transaction must pass with external storage");
+    }
 }

src/testing.rs

@@ -5,6 +5,7 @@
 
 #![cfg(test)]
 
+use crate::deps::InMemoryStorage;
 use crate::node::{InMemoryNode, TxExecutionInfo};
 use crate::{fork::ForkSource, node::compute_hash};
 
@@ -17,9 +18,12 @@ use httptest::{
 use itertools::Itertools;
 use multivm::interface::{ExecutionResult, VmExecutionResultAndLogs};
 use std::str::FromStr;
-use zksync_basic_types::{H160, U64};
-use zksync_types::api::{BridgeAddresses, DebugCall, DebugCallType, Log};
+use zksync_basic_types::{AccountTreeId, MiniblockNumber, H160, U64};
+use zksync_types::api::{BlockIdVariant, BridgeAddresses, DebugCall, DebugCallType, Log};
+use zksync_types::block::pack_block_info;
+use zksync_types::StorageKey;
 use zksync_types::{fee::Fee, l2::L2Tx, Address, L2ChainId, Nonce, ProtocolVersionId, H256, U256};
+use zksync_utils::u256_to_h256;
 
 /// Configuration for the [MockServer]'s initial block.
 #[derive(Default, Debug, Clone)]
@@ -668,7 +672,121 @@ pub fn assert_bridge_addresses_eq(
     );
 }
 
+/// Represents a read-only fork source that is backed by the provided [InMemoryStorage].
+#[derive(Debug, Clone)]
+pub struct ExternalStorage {
+    pub raw_storage: InMemoryStorage,
+}
+
+impl ForkSource for &ExternalStorage {
+    fn get_storage_at(
+        &self,
+        address: H160,
+        idx: U256,
+        _block: Option<BlockIdVariant>,
+    ) -> eyre::Result<H256> {
+        let key = StorageKey::new(AccountTreeId::new(address), u256_to_h256(idx));
+        Ok(self
+            .raw_storage
+            .state
+            .get(&key)
+            .cloned()
+            .unwrap_or_default())
+    }
+
+    fn get_raw_block_transactions(
+        &self,
+        _block_number: MiniblockNumber,
+    ) -> eyre::Result<Vec<zksync_types::Transaction>> {
+        todo!()
+    }
+
+    fn get_bytecode_by_hash(&self, hash: H256) -> eyre::Result<Option<Vec<u8>>> {
+        Ok(self.raw_storage.factory_deps.get(&hash).cloned())
+    }
+
+    fn get_transaction_by_hash(
+        &self,
+        _hash: H256,
+    ) -> eyre::Result<Option<zksync_types::api::Transaction>> {
+        todo!()
+    }
+
+    fn get_transaction_details(
+        &self,
+        _hash: H256,
+    ) -> eyre::Result<std::option::Option<zksync_types::api::TransactionDetails>> {
+        todo!()
+    }
+
+    fn get_block_by_hash(
+        &self,
+        _hash: H256,
+        _full_transactions: bool,
+    ) -> eyre::Result<Option<zksync_types::api::Block<zksync_types::api::TransactionVariant>>> {
+        todo!()
+    }
+
+    fn get_block_by_number(
+        &self,
+        _block_number: zksync_types::api::BlockNumber,
+        _full_transactions: bool,
+    ) -> eyre::Result<Option<zksync_types::api::Block<zksync_types::api::TransactionVariant>>> {
+        todo!()
+    }
+
+    fn get_block_details(
+        &self,
+        _miniblock: MiniblockNumber,
+    ) -> eyre::Result<Option<zksync_types::api::BlockDetails>> {
+        todo!()
+    }
+
+    fn get_block_transaction_count_by_hash(&self, _block_hash: H256) -> eyre::Result<Option<U256>> {
+        todo!()
+    }
+
+    fn get_block_transaction_count_by_number(
+        &self,
+        _block_number: zksync_types::api::BlockNumber,
+    ) -> eyre::Result<Option<U256>> {
+        todo!()
+    }
+
+    fn get_transaction_by_block_hash_and_index(
+        &self,
+        _block_hash: H256,
+        _index: zksync_basic_types::web3::types::Index,
+    ) -> eyre::Result<Option<zksync_types::api::Transaction>> {
+        todo!()
+    }
+
+    fn get_transaction_by_block_number_and_index(
+        &self,
+        _block_number: zksync_types::api::BlockNumber,
+        _index: zksync_basic_types::web3::types::Index,
+    ) -> eyre::Result<Option<zksync_types::api::Transaction>> {
+        todo!()
+    }
+
+    fn get_bridge_contracts(&self) -> eyre::Result<zksync_types::api::BridgeAddresses> {
+        todo!()
+    }
+
+    fn get_confirmed_tokens(
+        &self,
+        _from: u32,
+        _limit: u8,
+    ) -> eyre::Result<Vec<zksync_web3_decl::types::Token>> {
+        todo!()
+    }
+}
+
 mod test {
+    use maplit::hashmap;
+    use zksync_types::block::unpack_block_info;
+    use zksync_utils::h256_to_u256;
+
     use super::*;
     use crate::http_fork_source::HttpForkSource;
 
@@ -814,4 +932,73 @@ mod test {
             log.topics
         );
     }
+
+    #[test]
+    fn test_external_storage() {
+        let input_batch = 1;
+        let input_l2_block = 2;
+        let input_timestamp = 3;
+        let input_bytecode = vec![0x4];
+        let batch_key = StorageKey::new(
+            AccountTreeId::new(zksync_types::SYSTEM_CONTEXT_ADDRESS),
+            zksync_types::SYSTEM_CONTEXT_BLOCK_INFO_POSITION,
+        );
+        let l2_block_key = StorageKey::new(
+            AccountTreeId::new(zksync_types::SYSTEM_CONTEXT_ADDRESS),
+            zksync_types::SYSTEM_CONTEXT_CURRENT_L2_BLOCK_INFO_POSITION,
+        );
+
+        let storage = &ExternalStorage {
+            raw_storage: InMemoryStorage {
+                state: hashmap! {
+                    batch_key => u256_to_h256(U256::from(input_batch)),
+                    l2_block_key => u256_to_h256(pack_block_info(
+                        input_l2_block,
+                        input_timestamp,
+                    ))
+                },
+                factory_deps: hashmap! {
+                    H256::repeat_byte(0x1) => input_bytecode.clone(),
+                },
+            },
+        };
+
+        let actual_batch = storage
+            .get_storage_at(
+                zksync_types::SYSTEM_CONTEXT_ADDRESS,
+                h256_to_u256(zksync_types::SYSTEM_CONTEXT_BLOCK_INFO_POSITION),
+                None,
+            )
+            .map(|value| h256_to_u256(value).as_u64())
+            .expect("failed getting batch number");
+        assert_eq!(input_batch, actual_batch);
+
+        let (actual_l2_block, actual_timestamp) = storage
+            .get_storage_at(
+                zksync_types::SYSTEM_CONTEXT_ADDRESS,
+                h256_to_u256(zksync_types::SYSTEM_CONTEXT_CURRENT_L2_BLOCK_INFO_POSITION),
+                None,
+            )
+            .map(|value| unpack_block_info(h256_to_u256(value)))
+            .expect("failed getting l2 block info");
+        assert_eq!(input_l2_block, actual_l2_block);
+        assert_eq!(input_timestamp, actual_timestamp);
+
+        let zero_missing_value = storage
+            .get_storage_at(
+                zksync_types::SYSTEM_CONTEXT_ADDRESS,
+                h256_to_u256(H256::repeat_byte(0x1e)),
+                None,
+            )
+            .map(|value| h256_to_u256(value).as_u64())
+            .expect("failed missing value");
+        assert_eq!(0, zero_missing_value);
+
+        let actual_bytecode = storage
+            .get_bytecode_by_hash(H256::repeat_byte(0x1))
+            .ok()
+            .expect("failed getting bytecode")
+            .expect("missing bytecode");
+        assert_eq!(input_bytecode, actual_bytecode);
+    }
 }