Skip to content

Commit

Permalink
feat: move storage view, allow ignoring bootloader execution during t…
Browse files Browse the repository at this point in the history
…x execution (matter-labs#251)

* move storage view to test node

Signed-off-by: Danil <[email protected]>

* Update era

Signed-off-by: Danil <[email protected]>

* add modified keys and do not execute bootloader

Signed-off-by: Danil <[email protected]>

* Load l1 batch from local

Signed-off-by: Danil <[email protected]>

* Update era

Signed-off-by: Danil <[email protected]>

* Fix nits

Signed-off-by: Danil <[email protected]>

* Adapt tests

Signed-off-by: Danil <[email protected]>

* Introduce bool

Signed-off-by: Danil <[email protected]>

* Update era

Signed-off-by: Danil <[email protected]>

* remove modified keys

* fix test

* fix block production

---------

Signed-off-by: Danil <[email protected]>
Co-authored-by: Danil <[email protected]>
  • Loading branch information
nbaztec and Deniallugo authored Dec 22, 2023
1 parent 578cdbd commit 6ee7d29
Show file tree
Hide file tree
Showing 11 changed files with 336 additions and 262 deletions.
225 changes: 72 additions & 153 deletions Cargo.lock

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ categories = ["cryptography"]
publish = false # We don't want to publish our binaries.

[dependencies]
zksync_basic_types = { git = "https://github.com/matter-labs/zksync-era.git", rev = "bd268ac02bc3530c1d3247cb9496c3e13c2e52d9" }
zksync_core = { git = "https://github.com/matter-labs/zksync-era.git", rev = "bd268ac02bc3530c1d3247cb9496c3e13c2e52d9" }
multivm = { git = "https://github.com/matter-labs/zksync-era.git", rev = "bd268ac02bc3530c1d3247cb9496c3e13c2e52d9" }
zksync_contracts = { git = "https://github.com/matter-labs/zksync-era.git", rev = "bd268ac02bc3530c1d3247cb9496c3e13c2e52d9" }
zksync_types = { git = "https://github.com/matter-labs/zksync-era.git", rev = "bd268ac02bc3530c1d3247cb9496c3e13c2e52d9" }
zksync_utils = { git = "https://github.com/matter-labs/zksync-era.git", rev = "bd268ac02bc3530c1d3247cb9496c3e13c2e52d9" }
zksync_state = { git = "https://github.com/matter-labs/zksync-era.git", rev = "bd268ac02bc3530c1d3247cb9496c3e13c2e52d9" }
zksync_web3_decl = { git = "https://github.com/matter-labs/zksync-era.git", rev = "bd268ac02bc3530c1d3247cb9496c3e13c2e52d9" }
zksync_basic_types = { git = "https://github.com/matter-labs/zksync-era.git", rev = "3c669e7b0caf6515ad865f4cba5ea6fb36c33811" }
zksync_core = { git = "https://github.com/matter-labs/zksync-era.git", rev = "3c669e7b0caf6515ad865f4cba5ea6fb36c33811" }
multivm = { git = "https://github.com/matter-labs/zksync-era.git", rev = "3c669e7b0caf6515ad865f4cba5ea6fb36c33811" }
zksync_contracts = { git = "https://github.com/matter-labs/zksync-era.git", rev = "3c669e7b0caf6515ad865f4cba5ea6fb36c33811" }
zksync_types = { git = "https://github.com/matter-labs/zksync-era.git", rev = "3c669e7b0caf6515ad865f4cba5ea6fb36c33811" }
zksync_utils = { git = "https://github.com/matter-labs/zksync-era.git", rev = "3c669e7b0caf6515ad865f4cba5ea6fb36c33811" }
zksync_state = { git = "https://github.com/matter-labs/zksync-era.git", rev = "3c669e7b0caf6515ad865f4cba5ea6fb36c33811" }
zksync_web3_decl = { git = "https://github.com/matter-labs/zksync-era.git", rev = "3c669e7b0caf6515ad865f4cba5ea6fb36c33811" }
sha3 = "0.10.6"


Expand Down
4 changes: 2 additions & 2 deletions etc/system-contracts/bootloader/test_infra/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ fn execute_internal_bootloader_test() {
hash_bytecode,
get_system_smart_contracts_from_dir(env::current_dir().unwrap().join("../../")),
))
.to_rc_ptr();
.into_rc_ptr();

let mut vm = Vm::new(
l1_batch_env.clone(),
Expand Down Expand Up @@ -124,7 +124,7 @@ fn execute_internal_bootloader_test() {
hash_bytecode,
get_system_smart_contracts_from_dir(env::current_dir().unwrap().join("../../")),
))
.to_rc_ptr();
.into_rc_ptr();

// We are passing id of the test in location (0) where we normally put the operator.
// This is then picked up by the testing framework.
Expand Down
47 changes: 4 additions & 43 deletions src/deps/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use std::collections::HashMap;
pub mod system_contracts;
use zksync_types::{
get_code_key, get_known_code_key, get_system_context_init_logs, L2ChainId, StorageKey,
StorageLog, StorageLogKind, StorageValue, H256,
get_code_key, get_system_context_init_logs, L2ChainId, StorageKey, StorageLog, StorageLogKind,
StorageValue, H256,
};

use std::fmt;
pub mod storage_view;
use zksync_state::ReadStorage;

/// In-memory storage.
#[derive(Debug, Default, Clone, PartialEq)]
Expand Down Expand Up @@ -93,42 +93,3 @@ impl ReadStorage for InMemoryStorage {
(&*self).get_enumeration_index(key)
}
}

/// Functionality to read from the VM storage.
pub trait ReadStorage: fmt::Debug {
/// Read value of the key.
fn read_value(&mut self, key: &StorageKey) -> StorageValue;

/// Checks whether a write to this storage at the specified `key` would be an initial write.
/// Roughly speaking, this is the case when the storage doesn't contain `key`, although
/// in case of mutable storages, the caveats apply (a write to a key that is present
/// in the storage but was not committed is still an initial write).
fn is_write_initial(&mut self, key: &StorageKey) -> bool;

/// Load the factory dependency code by its hash.
fn load_factory_dep(&mut self, hash: H256) -> Option<Vec<u8>>;

/// Returns whether a bytecode hash is "known" to the system.
fn is_bytecode_known(&mut self, bytecode_hash: &H256) -> bool {
let code_key = get_known_code_key(bytecode_hash);
self.read_value(&code_key) != H256::zero()
}

/// Retrieves the enumeration index for a given `key`.
fn get_enumeration_index(&mut self, key: &StorageKey) -> Option<u64>;
}

/// Functionality to write to the VM storage in a batch.
///
/// So far, this trait is implemented only for [`zksync_state::StorageView`].
pub trait WriteStorage: ReadStorage {
/// Sets the new value under a given key and returns the previous value.
fn set_value(&mut self, key: StorageKey, value: StorageValue) -> StorageValue;

/// Returns a map with the key–value pairs updated by this batch.
fn modified_storage_keys(&self) -> &HashMap<StorageKey, StorageValue>;

/// Returns the number of read / write ops for which the value was read from the underlying
/// storage.
fn missed_storage_invocations(&self) -> usize;
}
164 changes: 164 additions & 0 deletions src/deps/storage_view.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
use std::{cell::RefCell, collections::HashMap, fmt, rc::Rc};

use zksync_state::{ReadStorage, WriteStorage};
use zksync_types::{StorageKey, StorageValue, H256};

/// `StorageView` is a buffer for `StorageLog`s between storage and transaction execution code.
/// In order to commit transactions logs should be submitted to the underlying storage
/// after a transaction is executed.
///
/// When executing transactions as a part of miniblock / L1 batch creation,
/// a single `StorageView` is used for the entire L1 batch.
/// One `StorageView` must not be used for multiple L1 batches;
/// otherwise, [`Self::is_write_initial()`] will return incorrect values because of the caching.
///
/// When executing transactions in the API sandbox, a dedicated view is used for each transaction;
/// the only shared part is the read storage keys cache.
#[derive(Debug)]
pub struct StorageView<S> {
pub storage_handle: S,
// Used for caching and to get the list/count of modified keys
pub modified_storage_keys: HashMap<StorageKey, StorageValue>,
// Used purely for caching
read_storage_keys: HashMap<StorageKey, StorageValue>,
// Cache for `contains_key()` checks. The cache is only valid within one L1 batch execution.
initial_writes_cache: HashMap<StorageKey, bool>,
}

impl<S: ReadStorage + fmt::Debug> StorageView<S> {
/// Creates a new storage view based on the underlying storage.
pub fn new(storage_handle: S) -> Self {
Self {
storage_handle,
modified_storage_keys: HashMap::new(),
read_storage_keys: HashMap::new(),
initial_writes_cache: HashMap::new(),
}
}

#[allow(dead_code)]
pub fn clean_cache(&mut self) {
self.modified_storage_keys = Default::default();
self.read_storage_keys = Default::default();
self.initial_writes_cache = Default::default();
}

fn get_value_no_log(&mut self, key: &StorageKey) -> StorageValue {
let cached_value = self
.modified_storage_keys
.get(key)
.or_else(|| self.read_storage_keys.get(key));
cached_value.copied().unwrap_or_else(|| {
let value = self.storage_handle.read_value(key);
self.read_storage_keys.insert(*key, value);
value
})
}
/// Make a Rc RefCell ptr to the storage
pub fn into_rc_ptr(self) -> Rc<RefCell<Self>> {
Rc::new(RefCell::new(self))
}
}

impl<S: ReadStorage + fmt::Debug> ReadStorage for StorageView<S> {
fn read_value(&mut self, key: &StorageKey) -> StorageValue {
let value = self.get_value_no_log(key);

tracing::trace!(
"read value {:?} {:?} ({:?}/{:?})",
key.hashed_key().0,
value.0,
key.address(),
key.key()
);

value
}

/// Only keys contained in the underlying storage will return `false`. If a key was
/// inserted using [`Self::set_value()`], it will still return `true`.
fn is_write_initial(&mut self, key: &StorageKey) -> bool {
if let Some(&is_write_initial) = self.initial_writes_cache.get(key) {
is_write_initial
} else {
let is_write_initial = self.storage_handle.is_write_initial(key);
self.initial_writes_cache.insert(*key, is_write_initial);
is_write_initial
}
}

fn load_factory_dep(&mut self, hash: H256) -> Option<Vec<u8>> {
self.storage_handle.load_factory_dep(hash)
}

fn get_enumeration_index(&mut self, key: &StorageKey) -> Option<u64> {
self.storage_handle.get_enumeration_index(key)
}
}

impl<S: ReadStorage + fmt::Debug> WriteStorage for StorageView<S> {
fn set_value(&mut self, key: StorageKey, value: StorageValue) -> StorageValue {
let original = self.get_value_no_log(&key);

tracing::trace!(
"write value {:?} value: {:?} original value: {:?} ({:?}/{:?})",
key.hashed_key().0,
value,
original,
key.address(),
key.key()
);
self.modified_storage_keys.insert(key, value);

original
}

fn modified_storage_keys(&self) -> &HashMap<StorageKey, StorageValue> {
&self.modified_storage_keys
}

fn missed_storage_invocations(&self) -> usize {
0
}
}

#[cfg(test)]
mod test {
use super::*;
use crate::deps::InMemoryStorage;
use zksync_types::{AccountTreeId, Address, H256};

#[test]
fn test_storage_access() {
let account: AccountTreeId = AccountTreeId::new(Address::from([0xfe; 20]));
let key = H256::from_low_u64_be(61);
let value = H256::from_low_u64_be(73);
let key = StorageKey::new(account, key);

let mut raw_storage = InMemoryStorage::default();
let mut storage_view = StorageView::new(&raw_storage);

let default_value = storage_view.read_value(&key);
assert_eq!(default_value, H256::zero());

let prev_value = storage_view.set_value(key, value);
assert_eq!(prev_value, H256::zero());
assert_eq!(storage_view.read_value(&key), value);
assert!(storage_view.is_write_initial(&key)); // key was inserted during the view lifetime

raw_storage.set_value(key, value);
let mut storage_view = StorageView::new(&raw_storage);

assert_eq!(storage_view.read_value(&key), value);
assert!(!storage_view.is_write_initial(&key)); // `key` is present in `raw_storage`

let new_value = H256::from_low_u64_be(74);
storage_view.set_value(key, new_value);
assert_eq!(storage_view.read_value(&key), new_value);

let new_key = StorageKey::new(account, H256::from_low_u64_be(62));
storage_view.set_value(new_key, new_value);
assert_eq!(storage_view.read_value(&new_key), new_value);
assert!(storage_view.is_write_initial(&new_key));
}
}
2 changes: 1 addition & 1 deletion src/fork.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ use zksync_web3_decl::{
};
use zksync_web3_decl::{jsonrpsee::http_client::HttpClientBuilder, namespaces::ZksNamespaceClient};

use crate::system_contracts;
use crate::{cache::CacheConfig, node::TEST_NODE_NETWORK_ID};
use crate::{deps::InMemoryStorage, http_fork_source::HttpForkSource};
use crate::{deps::ReadStorage as RS, system_contracts};

pub fn block_on<F: Future + Send + 'static>(future: F) -> F::Output
where
Expand Down
11 changes: 3 additions & 8 deletions src/namespaces/debug.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
use crate::{
fork::ForkSource,
node::{InMemoryNodeInner, MAX_TX_SIZE},
utils::{create_debug_output, to_real_block_number},
utils::{create_debug_output, storage_view::StorageView, to_real_block_number},
};
use itertools::Itertools;
use jsonrpc_core::{BoxFuture, Result};
use multivm::vm_latest::HistoryDisabled;
use multivm::interface::VmInterface;
use multivm::vm_latest::{constants::ETH_CALL_GAS_LIMIT, CallTracer, Vm};
use once_cell::sync::OnceCell;
Expand All @@ -14,7 +13,6 @@ use zksync_basic_types::H256;
use zksync_core::api_server::web3::backend_jsonrpc::{
error::into_jsrpc_error, namespaces::debug::DebugNamespaceT,
};
use zksync_state::StorageView;
use zksync_types::{
api::{BlockId, BlockNumber, DebugCall, ResultDebugCall, TracerConfig, TransactionVariant},
l2::L2Tx,
Expand Down Expand Up @@ -172,7 +170,7 @@ impl<S: Send + Sync + 'static + ForkSource + std::fmt::Debug> DebugNamespaceT
};

let execution_mode = multivm::interface::TxExecutionMode::EthCall;
let storage = StorageView::new(&inner.fork_storage).to_rc_ptr();
let storage = StorageView::new(&inner.fork_storage).into_rc_ptr();

let bootloader_code = inner.system_contracts.contracts_for_l2_call();

Expand Down Expand Up @@ -200,10 +198,7 @@ impl<S: Send + Sync + 'static + ForkSource + std::fmt::Debug> DebugNamespaceT

let call_tracer_result = Arc::new(OnceCell::default());
let tracer = CallTracer::new(call_tracer_result.clone()).into_tracer_pointer();
let tx_result = vm.inspect(
tracer.into(),
multivm::interface::VmExecutionMode::OneTx,
);
let tx_result = vm.inspect(tracer.into(), multivm::interface::VmExecutionMode::OneTx);

let call_traces = if only_top {
vec![]
Expand Down
4 changes: 2 additions & 2 deletions src/node/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use multivm::vm_latest::{constants::ETH_CALL_GAS_LIMIT, ToTracerPointer, Vm};

use zksync_basic_types::H256;
use zksync_core::api_server::web3::backend_jsonrpc::error::into_jsrpc_error;
use zksync_state::StorageView;
use zksync_types::{
api::{BlockId, BlockNumber, DebugCall, ResultDebugCall, TracerConfig, TransactionVariant},
l2::L2Tx,
Expand All @@ -18,6 +17,7 @@ use zksync_types::{
};
use zksync_web3_decl::error::Web3Error;

use crate::deps::storage_view::StorageView;
use crate::{
fork::ForkSource,
namespaces::{DebugNamespaceT, Result, RpcResult},
Expand Down Expand Up @@ -162,7 +162,7 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> DebugNames
};

let execution_mode = multivm::interface::TxExecutionMode::EthCall;
let storage = StorageView::new(&inner.fork_storage).to_rc_ptr();
let storage = StorageView::new(&inner.fork_storage).into_rc_ptr();

let bootloader_code = inner.system_contracts.contracts_for_l2_call();

Expand Down
Loading

0 comments on commit 6ee7d29

Please sign in to comment.