Skip to content

Commit

Permalink
fix: added errors to internal implementation of ReadStorage (#292)
Browse files Browse the repository at this point in the history
  • Loading branch information
@vbar
vbar committed Jun 25, 2024
1 parent 8f60d51 commit 2832bef
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 73 deletions.
62 changes: 31 additions & 31 deletions src/fork.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,77 +100,77 @@ impl<S: ForkSource> ForkStorage<S> {
}
}

fn read_value_internal(&self, key: &StorageKey) -> zksync_types::StorageValue {
pub fn read_value_internal(
&self,
key: &StorageKey,
) -> eyre::Result<zksync_types::StorageValue> {
let mut mutator = self.inner.write().unwrap();
let local_storage = mutator.raw_storage.read_value(key);

if let Some(fork) = &mutator.fork {
if !H256::is_zero(&local_storage) {
return local_storage;
return Ok(local_storage);
}

if let Some(value) = mutator.value_read_cache.get(key) {
return *value;
return Ok(*value);
}
let l2_miniblock = fork.l2_miniblock;
let key_ = *key;

let result = fork
.fork_source
.get_storage_at(
*key_.account().address(),
h256_to_u256(*key_.key()),
Some(BlockIdVariant::BlockNumber(BlockNumber::Number(U64::from(
l2_miniblock,
)))),
)
.unwrap();
let result = fork.fork_source.get_storage_at(
*key_.account().address(),
h256_to_u256(*key_.key()),
Some(BlockIdVariant::BlockNumber(BlockNumber::Number(U64::from(
l2_miniblock,
)))),
)?;

mutator.value_read_cache.insert(*key, result);
result
Ok(result)
} else {
local_storage
Ok(local_storage)
}
}

fn load_factory_dep_internal(&self, hash: H256) -> Option<Vec<u8>> {
pub fn load_factory_dep_internal(&self, hash: H256) -> eyre::Result<Option<Vec<u8>>> {
let mut mutator = self.inner.write().unwrap();
let local_storage = mutator.raw_storage.load_factory_dep(hash);
if let Some(fork) = &mutator.fork {
if local_storage.is_some() {
return local_storage;
return Ok(local_storage);
}
if let Some(value) = mutator.factory_dep_cache.get(&hash) {
return value.clone();
return Ok(value.clone());
}

let result = fork.fork_source.get_bytecode_by_hash(hash).unwrap();
let result = fork.fork_source.get_bytecode_by_hash(hash)?;
mutator.factory_dep_cache.insert(hash, result.clone());
result
Ok(result)
} else {
local_storage
Ok(local_storage)
}
}

/// Check if this is the first time when we're ever writing to this key.
/// This has impact on amount of pubdata that we have to spend for the write.
fn is_write_initial_internal(&self, key: &StorageKey) -> bool {
pub fn is_write_initial_internal(&self, key: &StorageKey) -> eyre::Result<bool> {
// Currently we don't have the zks API to return us the information on whether a given
// key was written to before a given block.
// This means, we have to depend on the following heuristic: we'll read the value of the slot.
// - if value != 0 -> this means that the slot was written to in the past (so we can return intitial_write = false)
// - but if the value = 0 - there is a chance, that slot was written to in the past - and later was reset.
// but unfortunately we cannot detect that with the current zks api, so we'll attempt to do it
// only on local storage.
let value = self.read_value_internal(key);
let value = self.read_value_internal(key)?;
if value != H256::zero() {
return false;
return Ok(false);
}

// If value was 0, there is still a chance, that the slot was written to in the past - and only now set to 0.
// We unfortunately don't have the API to check it on the fork, but we can at least try to check it on local storage.
let mut mutator = self.inner.write().unwrap();
mutator.raw_storage.is_write_initial(key)
Ok(mutator.raw_storage.is_write_initial(key))
}

/// Retrieves the enumeration index for a given `key`.
Expand All @@ -182,15 +182,15 @@ impl<S: ForkSource> ForkStorage<S> {

impl<S: std::fmt::Debug + ForkSource> ReadStorage for ForkStorage<S> {
fn is_write_initial(&mut self, key: &StorageKey) -> bool {
self.is_write_initial_internal(key)
self.is_write_initial_internal(key).unwrap()
}

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

fn read_value(&mut self, key: &StorageKey) -> zksync_types::StorageValue {
self.read_value_internal(key)
self.read_value_internal(key).unwrap()
}

fn get_enumeration_index(&mut self, key: &StorageKey) -> Option<u64> {
Expand All @@ -200,15 +200,15 @@ impl<S: std::fmt::Debug + ForkSource> ReadStorage for ForkStorage<S> {

impl<S: std::fmt::Debug + ForkSource> ReadStorage for &ForkStorage<S> {
fn read_value(&mut self, key: &StorageKey) -> zksync_types::StorageValue {
self.read_value_internal(key)
self.read_value_internal(key).unwrap()
}

fn is_write_initial(&mut self, key: &StorageKey) -> bool {
self.is_write_initial_internal(key)
self.is_write_initial_internal(key).unwrap()
}

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

fn get_enumeration_index(&mut self, key: &StorageKey) -> Option<u64> {
Expand Down
56 changes: 34 additions & 22 deletions src/node/eth.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use zksync_basic_types::{
web3::{self, Bytes},
AccountTreeId, Address, H160, H256, U256, U64,
};
use zksync_state::ReadStorage;
use zksync_types::{
api::{Block, BlockIdVariant, BlockNumber, TransactionVariant},
fee::Fee,
Expand All @@ -30,7 +29,10 @@ use crate::{
fork::ForkSource,
namespaces::{EthNamespaceT, EthTestNodeNamespaceT, RpcResult},
node::{InMemoryNode, TransactionResult, MAX_TX_SIZE, PROTOCOL_VERSION},
utils::{self, h256_to_u64, into_jsrpc_error, not_implemented, IntoBoxedFuture},
utils::{
self, h256_to_u64, into_jsrpc_error, not_implemented, report_into_jsrpc_error,
IntoBoxedFuture,
},
};

impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> EthNamespaceT
Expand Down Expand Up @@ -137,9 +139,11 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> EthNamespa
);

match inner.write() {
Ok(mut inner_guard) => {
let balance = inner_guard.fork_storage.read_value(&balance_key);
Ok(h256_to_u256(balance))
Ok(inner_guard) => {
match inner_guard.fork_storage.read_value_internal(&balance_key) {
Ok(balance) => Ok(h256_to_u256(balance)),
Err(error) => Err(report_into_jsrpc_error(error)),
}
}
Err(_) => {
let error_message = "Error acquiring write lock for balance retrieval";
Expand Down Expand Up @@ -259,16 +263,18 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> EthNamespa
let code_key = get_code_key(&address);

match inner.write() {
Ok(mut guard) => {
let code_hash = guard.fork_storage.read_value(&code_key);

let code = guard
.fork_storage
.load_factory_dep(code_hash)
.unwrap_or_default();

Ok(Bytes::from(code))
}
Ok(guard) => match guard.fork_storage.read_value_internal(&code_key) {
Ok(code_hash) => {
match guard.fork_storage.load_factory_dep_internal(code_hash) {
Ok(raw_code) => {
let code = raw_code.unwrap_or_default();
Ok(Bytes::from(code))
}
Err(error) => Err(report_into_jsrpc_error(error)),
}
}
Err(error) => Err(report_into_jsrpc_error(error)),
},
Err(_) => Err(into_jsrpc_error(Web3Error::InternalError(
anyhow::Error::msg("Failed to acquire write lock for code retrieval"),
))),
Expand Down Expand Up @@ -297,10 +303,10 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> EthNamespa
let nonce_key = get_nonce_key(&address);

match inner.write() {
Ok(mut guard) => {
let result = guard.fork_storage.read_value(&nonce_key);
Ok(h256_to_u64(result).into())
}
Ok(guard) => match guard.fork_storage.read_value_internal(&nonce_key) {
Ok(result) => Ok(h256_to_u64(result).into()),
Err(error) => Err(report_into_jsrpc_error(error)),
},
Err(_) => Err(into_jsrpc_error(Web3Error::InternalError(
anyhow::Error::msg("Failed to acquire write lock for nonce retrieval"),
))),
Expand Down Expand Up @@ -1020,7 +1026,7 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> EthNamespa
let inner = self.get_inner().clone();

Box::pin(async move {
let mut writer = match inner.write() {
let writer = match inner.write() {
Ok(r) => r,
Err(_) => {
return Err(into_jsrpc_error(Web3Error::InternalError(
Expand Down Expand Up @@ -1058,7 +1064,10 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> EthNamespa
.unwrap_or_else(|| Ok(U64::from(writer.current_miniblock)))?;

if block_number.as_u64() == writer.current_miniblock {
Ok(H256(writer.fork_storage.read_value(&storage_key).0))
match writer.fork_storage.read_value_internal(&storage_key) {
Ok(value) => Ok(H256(value.0)),
Err(error) => Err(report_into_jsrpc_error(error)),
}
} else if writer.block_hashes.contains_key(&block_number.as_u64()) {
let value = writer
.block_hashes
Expand All @@ -1069,7 +1078,10 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> EthNamespa
.unwrap_or_default();

if value.is_zero() {
Ok(H256(writer.fork_storage.read_value(&storage_key).0))
match writer.fork_storage.read_value_internal(&storage_key) {
Ok(value) => Ok(H256(value.0)),
Err(error) => Err(report_into_jsrpc_error(error)),
}
} else {
Ok(value)
}
Expand Down
8 changes: 6 additions & 2 deletions src/node/in_memory_ext.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use anyhow::anyhow;
use zksync_basic_types::{Address, U256, U64};
use zksync_state::ReadStorage;
use zksync_types::{
get_code_key, get_nonce_key,
utils::{decompose_full_nonce, nonces_to_full_nonce, storage_key_for_eth_balance},
Expand Down Expand Up @@ -210,7 +209,12 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> InMemoryNo
.map_err(|err| anyhow!("failed acquiring lock: {:?}", err))
.and_then(|mut writer| {
let nonce_key = get_nonce_key(&address);
let full_nonce = writer.fork_storage.read_value(&nonce_key);
let full_nonce = match writer.fork_storage.read_value_internal(&nonce_key) {
Ok(full_nonce) => full_nonce,
Err(error) => {
return Err(anyhow!(error.to_string()));
}
};
let (mut account_nonce, mut deployment_nonce) =
decompose_full_nonce(h256_to_u256(full_nonce));
if account_nonce >= nonce {
Expand Down
57 changes: 39 additions & 18 deletions src/node/zks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use bigdecimal::BigDecimal;
use colored::Colorize;
use futures::FutureExt;
use zksync_basic_types::{AccountTreeId, Address, L1BatchNumber, L2BlockNumber, H256, U256};
use zksync_state::ReadStorage;
use zksync_types::{
api::{
BlockDetails, BlockDetailsBase, BlockStatus, BridgeAddresses, Proof, ProtocolVersion,
Expand All @@ -22,8 +21,8 @@ use crate::{
namespaces::{RpcResult, ZksNamespaceT},
node::{InMemoryNode, TransactionResult},
utils::{
internal_error, into_jsrpc_error, not_implemented, utc_datetime_from_epoch_ms,
IntoBoxedFuture,
internal_error, into_jsrpc_error, not_implemented, report_into_jsrpc_error,
utc_datetime_from_epoch_ms, IntoBoxedFuture,
},
};

Expand Down Expand Up @@ -295,7 +294,7 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> ZksNamespa
})?;

let balances = {
let mut writer = inner.write().map_err(|_e| {
let writer = inner.write().map_err(|_e| {
let error_message = "Failed to acquire lock. Please ensure the lock is not being held by another process or thread.".to_string();
into_jsrpc_error(Web3Error::InternalError(anyhow::Error::msg(error_message)))
})?;
Expand All @@ -305,7 +304,12 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> ZksNamespa
AccountTreeId::new(token.l2_address),
&address,
);
let balance = writer.fork_storage.read_value(&balance_key);
let balance = match writer.fork_storage.read_value_internal(&balance_key) {
Ok(balance) => balance,
Err(error) => {
return Err(report_into_jsrpc_error(error));
}
};
if !balance.is_zero() {
balances.insert(token.l2_address, h256_to_u256(balance));
}
Expand Down Expand Up @@ -513,24 +517,41 @@ impl<S: ForkSource + std::fmt::Debug + Clone + Send + Sync + 'static> ZksNamespa
fn get_bytecode_by_hash(&self, hash: zksync_basic_types::H256) -> RpcResult<Option<Vec<u8>>> {
let inner = self.get_inner().clone();
Box::pin(async move {
let mut writer = inner.write().map_err(|_e| {
let writer = inner.write().map_err(|_e| {
into_jsrpc_error(Web3Error::InternalError(anyhow::Error::msg(
"Failed to acquire write lock for bytecode retrieval.",
)))
})?;

let maybe_bytecode = writer.fork_storage.load_factory_dep(hash).or_else(|| {
writer
.fork_storage
.inner
.read()
.expect("failed reading fork storage")
.fork
.as_ref()
.and_then(|fork| fork.fork_source.get_bytecode_by_hash(hash).ok().flatten())
});

Ok(maybe_bytecode)
let maybe_bytecode = match writer.fork_storage.load_factory_dep_internal(hash) {
Ok(maybe_bytecode) => maybe_bytecode,
Err(error) => {
return Err(report_into_jsrpc_error(error));
}
};

if maybe_bytecode.is_some() {
return Ok(maybe_bytecode);
}

let maybe_fork_details = &writer
.fork_storage
.inner
.read()
.expect("failed reading fork storage")
.fork;
if let Some(fork_details) = maybe_fork_details {
let maybe_bytecode = match fork_details.fork_source.get_bytecode_by_hash(hash) {
Ok(maybe_bytecode) => maybe_bytecode,
Err(error) => {
return Err(report_into_jsrpc_error(error));
}
};

Ok(maybe_bytecode)
} else {
Ok(None)
}
})
}

Expand Down
6 changes: 6 additions & 0 deletions src/utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,12 @@ pub fn utc_datetime_from_epoch_ms(millis: u64) -> DateTime<Utc> {
DateTime::<Utc>::from_timestamp(secs as i64, nanos as u32).expect("valid timestamp")
}

pub fn report_into_jsrpc_error(error: eyre::Report) -> Error {
into_jsrpc_error(Web3Error::InternalError(anyhow::Error::msg(
error.to_string(),
)))
}

pub fn into_jsrpc_error(err: Web3Error) -> Error {
Error {
code: match err {
Expand Down

0 comments on commit 2832bef

Please sign in to comment.