From a74c39c0d89aec29086524f9a8dd13bf2e4e03f6 Mon Sep 17 00:00:00 2001 From: Nisheeth Barthwal Date: Tue, 7 Nov 2023 23:56:21 +0530 Subject: [PATCH] fix: rename run_l2_tx_inner, fix panics for lib users (#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 | 2 +- src/node/in_memory.rs | 73 +++++++++++++--- src/testing.rs | 191 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 252 insertions(+), 14 deletions(-) diff --git a/src/node/eth.rs b/src/node/eth.rs index fa478ca8..2c7cf4b7 100644 --- a/src/node/eth.rs +++ b/src/node/eth.rs @@ -814,7 +814,7 @@ impl 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 { diff --git a/src/node/in_memory.rs b/src/node/in_memory.rs index 6e7d170d..fe153c06 100644 --- a/src/node/in_memory.rs +++ b/src/node/in_memory.rs @@ -1253,7 +1253,26 @@ impl InMemoryNode { } /// 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 InMemoryNode { 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 InMemoryNode { } 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::::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"); + } } diff --git a/src/testing.rs b/src/testing.rs index 07346be5..4e7a3609 100644 --- a/src/testing.rs +++ b/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, + ) -> eyre::Result { + 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> { + todo!() + } + + fn get_bytecode_by_hash(&self, hash: H256) -> eyre::Result>> { + Ok(self.raw_storage.factory_deps.get(&hash).cloned()) + } + + fn get_transaction_by_hash( + &self, + _hash: H256, + ) -> eyre::Result> { + todo!() + } + + fn get_transaction_details( + &self, + _hash: H256, + ) -> eyre::Result> { + todo!() + } + + fn get_block_by_hash( + &self, + _hash: H256, + _full_transactions: bool, + ) -> eyre::Result>> { + todo!() + } + + fn get_block_by_number( + &self, + _block_number: zksync_types::api::BlockNumber, + _full_transactions: bool, + ) -> eyre::Result>> { + todo!() + } + + fn get_block_details( + &self, + _miniblock: MiniblockNumber, + ) -> eyre::Result> { + todo!() + } + + fn get_block_transaction_count_by_hash(&self, _block_hash: H256) -> eyre::Result> { + todo!() + } + + fn get_block_transaction_count_by_number( + &self, + _block_number: zksync_types::api::BlockNumber, + ) -> eyre::Result> { + todo!() + } + + fn get_transaction_by_block_hash_and_index( + &self, + _block_hash: H256, + _index: zksync_basic_types::web3::types::Index, + ) -> eyre::Result> { + 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> { + todo!() + } + + fn get_bridge_contracts(&self) -> eyre::Result { + todo!() + } + + fn get_confirmed_tokens( + &self, + _from: u32, + _limit: u8, + ) -> eyre::Result> { + 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); + } }