Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: added errors to internal implementation of ReadStorage #292

Merged
merged 3 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 22 additions & 23 deletions src/fork.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,22 +97,22 @@
factory_dep_cache: Default::default(),
})),
chain_id,
}

Check warning on line 100 in src/fork.rs

View workflow job for this annotation

GitHub Actions / lint

Diff in /home/runner/work/era-test-node/era-test-node/src/fork.rs
}

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;

Check warning on line 115 in src/fork.rs

View workflow job for this annotation

GitHub Actions / lint

Diff in /home/runner/work/era-test-node/era-test-node/src/fork.rs
let key_ = *key;

let result = fork
Expand All @@ -123,54 +123,53 @@
Some(BlockIdVariant::BlockNumber(BlockNumber::Number(U64::from(
l2_miniblock,
)))),
)
.unwrap();
)?;

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 +181,15 @@

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()
MexicanAce marked this conversation as resolved.
Show resolved Hide resolved
}

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 +199,15 @@

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
58 changes: 38 additions & 20 deletions src/node/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
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 @@ -27,10 +26,10 @@

use crate::{
filters::{FilterType, LogFilter},
fork::ForkSource,

Check warning on line 29 in src/node/eth.rs

View workflow job for this annotation

GitHub Actions / lint

Diff in /home/runner/work/era-test-node/era-test-node/src/node/eth.rs
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 +136,13 @@
);

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) => {

Check warning on line 139 in src/node/eth.rs

View workflow job for this annotation

GitHub Actions / lint

Diff in /home/runner/work/era-test-node/era-test-node/src/node/eth.rs
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 @@ -256,18 +259,24 @@
let inner = self.get_inner().clone();

Box::pin(async move {
let code_key = get_code_key(&address);

Check warning on line 262 in src/node/eth.rs

View workflow job for this annotation

GitHub Actions / lint

Diff in /home/runner/work/era-test-node/era-test-node/src/node/eth.rs

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 All @@ -294,12 +303,15 @@
let inner = self.get_inner().clone();

Box::pin(async move {
let nonce_key = get_nonce_key(&address);

Check warning on line 306 in src/node/eth.rs

View workflow job for this annotation

GitHub Actions / lint

Diff in /home/runner/work/era-test-node/era-test-node/src/node/eth.rs

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 +1032,7 @@
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 +1070,10 @@
.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 +1084,10 @@
.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
6 changes: 4 additions & 2 deletions src/node/in_memory_ext.rs
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 @@ -209,8 +208,11 @@
.write()
.map_err(|err| anyhow!("failed acquiring lock: {:?}", err))
.and_then(|mut writer| {
let nonce_key = get_nonce_key(&address);

Check warning on line 211 in src/node/in_memory_ext.rs

View workflow job for this annotation

GitHub Actions / lint

Diff in /home/runner/work/era-test-node/era-test-node/src/node/in_memory_ext.rs
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
51 changes: 34 additions & 17 deletions src/node/zks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
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 @@ -19,10 +18,10 @@

use crate::{
fork::ForkSource,
namespaces::{RpcResult, ZksNamespaceT},

Check warning on line 21 in src/node/zks.rs

View workflow job for this annotation

GitHub Actions / lint

Diff in /home/runner/work/era-test-node/era-test-node/src/node/zks.rs
node::{InMemoryNode, TransactionResult},
utils::{
internal_error, into_jsrpc_error, not_implemented, utc_datetime_from_epoch_ms,
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 @@
})?;

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 @@
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,37 @@
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.",
)))
})?;

Check warning on line 525 in src/node/zks.rs

View workflow job for this annotation

GitHub Actions / lint

Diff in /home/runner/work/era-test-node/era-test-node/src/node/zks.rs
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 {

Check warning on line 541 in src/node/zks.rs

View workflow job for this annotation

GitHub Actions / lint

Diff in /home/runner/work/era-test-node/era-test-node/src/node/zks.rs
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
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
Loading