Skip to content

Commit

Permalink
Fix nits
Browse files Browse the repository at this point in the history
Signed-off-by: Danil <deniallugo@gmail.com>
  • Loading branch information
Deniallugo committed Dec 18, 2023
1 parent d527b98 commit 5e7dea4
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 66 deletions.
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
99 changes: 50 additions & 49 deletions src/deps/storage_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ impl<S: ReadStorage + fmt::Debug> StorageView<S> {
}
}

#[allow(dead_code)]
pub fn clean_cache(&mut self) {
self.modified_storage_keys = Default::default();
self.read_storage_keys = Default::default();
Expand All @@ -54,7 +55,7 @@ impl<S: ReadStorage + fmt::Debug> StorageView<S> {
})
}
/// Make a Rc RefCell ptr to the storage
pub fn to_rc_ptr(self) -> Rc<RefCell<Self>> {
pub fn into_rc_ptr(self) -> Rc<RefCell<Self>> {
Rc::new(RefCell::new(self))
}
}
Expand Down Expand Up @@ -121,51 +122,51 @@ impl<S: ReadStorage + fmt::Debug> WriteStorage for StorageView<S> {
}
}

// #[cfg(test)]
// mod test {
// use super::*;
// 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

// assert_eq!(storage_view.metrics().storage_invocations_missed, 1);
// // ^ We should only read a value at `key` once, and then used the cached value.

// 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));

// let metrics = storage_view.metrics();
// assert_eq!(metrics.storage_invocations_missed, 2);
// assert_eq!(metrics.get_value_storage_invocations, 3);
// assert_eq!(metrics.set_value_storage_invocations, 2);
// }
// }
#[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);

Check failure on line 139 in src/deps/storage_view.rs

View workflow job for this annotation

GitHub Actions / unit-tests (ubuntu-22.04-github-hosted-16core)

the trait bound `&deps::InMemoryStorage: zksync_state::ReadStorage` is not satisfied

let default_value = storage_view.read_value(&key);

Check failure on line 141 in src/deps/storage_view.rs

View workflow job for this annotation

GitHub Actions / unit-tests (ubuntu-22.04-github-hosted-16core)

the method `read_value` exists for struct `StorageView<&InMemoryStorage>`, but its trait bounds were not satisfied
assert_eq!(default_value, H256::zero());

let prev_value = storage_view.set_value(key, value);

Check failure on line 144 in src/deps/storage_view.rs

View workflow job for this annotation

GitHub Actions / unit-tests (ubuntu-22.04-github-hosted-16core)

the method `set_value` exists for struct `StorageView<&InMemoryStorage>`, but its trait bounds were not satisfied
assert_eq!(prev_value, H256::zero());
assert_eq!(storage_view.read_value(&key), value);

Check failure on line 146 in src/deps/storage_view.rs

View workflow job for this annotation

GitHub Actions / unit-tests (ubuntu-22.04-github-hosted-16core)

the method `read_value` exists for struct `StorageView<&InMemoryStorage>`, but its trait bounds were not satisfied
assert!(storage_view.is_write_initial(&key)); // key was inserted during the view lifetime

Check failure on line 147 in src/deps/storage_view.rs

View workflow job for this annotation

GitHub Actions / unit-tests (ubuntu-22.04-github-hosted-16core)

the method `is_write_initial` exists for struct `StorageView<&InMemoryStorage>`, but its trait bounds were not satisfied

assert_eq!(storage_view.metrics().storage_invocations_missed, 1);

Check failure on line 149 in src/deps/storage_view.rs

View workflow job for this annotation

GitHub Actions / unit-tests (ubuntu-22.04-github-hosted-16core)

no method named `metrics` found for struct `storage_view::StorageView` in the current scope
// ^ We should only read a value at `key` once, and then used the cached value.

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

Check failure on line 153 in src/deps/storage_view.rs

View workflow job for this annotation

GitHub Actions / unit-tests (ubuntu-22.04-github-hosted-16core)

the trait bound `&deps::InMemoryStorage: zksync_state::ReadStorage` is not satisfied

assert_eq!(storage_view.read_value(&key), value);

Check failure on line 155 in src/deps/storage_view.rs

View workflow job for this annotation

GitHub Actions / unit-tests (ubuntu-22.04-github-hosted-16core)

the method `read_value` exists for struct `StorageView<&InMemoryStorage>`, but its trait bounds were not satisfied
assert!(!storage_view.is_write_initial(&key)); // `key` is present in `raw_storage`

Check failure on line 156 in src/deps/storage_view.rs

View workflow job for this annotation

GitHub Actions / unit-tests (ubuntu-22.04-github-hosted-16core)

the method `is_write_initial` exists for struct `StorageView<&InMemoryStorage>`, but its trait bounds were not satisfied

let new_value = H256::from_low_u64_be(74);
storage_view.set_value(key, new_value);

Check failure on line 159 in src/deps/storage_view.rs

View workflow job for this annotation

GitHub Actions / unit-tests (ubuntu-22.04-github-hosted-16core)

the method `set_value` exists for struct `StorageView<&InMemoryStorage>`, but its trait bounds were not satisfied
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));

let metrics = storage_view.metrics();
assert_eq!(metrics.storage_invocations_missed, 2);
assert_eq!(metrics.get_value_storage_invocations, 3);
assert_eq!(metrics.set_value_storage_invocations, 2);
}
}
2 changes: 1 addition & 1 deletion src/namespaces/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,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
2 changes: 1 addition & 1 deletion src/node/debug.rs
Original file line number Diff line number Diff line change
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
17 changes: 5 additions & 12 deletions src/node/in_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,15 +341,13 @@ impl<S: std::fmt::Debug + ForkSource> InMemoryNodeInner<S> {
max_virtual_blocks_to_create: 1,
}
} else {
// This is the scenario of either the first L2 block ever
L2BlockEnv {
number: 1,
timestamp: 1,
prev_block_hash: MiniblockHasher::legacy_hash(MiniblockNumber(0)),
max_virtual_blocks_to_create: 1,
}
// This is the scenario of either the first L2 block ever or
// the first block after the upgrade for support of L2 blocks.
// legacy_miniblock_hash(MiniblockNumber(self.current_miniblock as u32))
};
let block_ctx = if let Some((batch_number, batch_timestamp)) = load_last_l1_batch(storage) {
BlockContext::from_current(
Expand Down Expand Up @@ -502,7 +500,7 @@ impl<S: std::fmt::Debug + ForkSource> InMemoryNodeInner<S> {
let gas_for_bytecodes_pubdata: u32 =
pubdata_for_factory_deps * (gas_per_pubdata_byte as u32);

let storage = storage_view.to_rc_ptr();
let storage = storage_view.into_rc_ptr();

let execution_mode = TxExecutionMode::EstimateFee;
let (mut batch_env, _) = self.create_l1_batch_env(storage.clone());
Expand Down Expand Up @@ -708,7 +706,7 @@ impl<S: std::fmt::Debug + ForkSource> InMemoryNodeInner<S> {
);
l2_tx.common_data.fee.gas_limit = gas_limit_with_overhead.into();

let storage = StorageView::new(fork_storage).to_rc_ptr();
let storage = StorageView::new(fork_storage).into_rc_ptr();

// The nonce needs to be updated
let nonce = l2_tx.nonce();
Expand Down Expand Up @@ -1016,7 +1014,7 @@ impl<S: ForkSource + std::fmt::Debug + Clone> InMemoryNode<S> {
.write()
.map_err(|e| format!("Failed to acquire write lock: {}", e))?;

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 @@ -1300,11 +1298,10 @@ impl<S: ForkSource + std::fmt::Debug + Clone> InMemoryNode<S> {
.write()
.map_err(|e| format!("Failed to acquire write lock: {}", e))?;

let storage = StorageView::new(inner.fork_storage.clone()).to_rc_ptr();
let storage = StorageView::new(inner.fork_storage.clone()).into_rc_ptr();
storage.borrow_mut().modified_storage_keys = modified_storage_keys;

let (batch_env, block_ctx) = inner.create_l1_batch_env(storage.clone());
dbg!(&batch_env);

let bootloader_code = {
if inner
Expand Down Expand Up @@ -1472,16 +1469,12 @@ impl<S: ForkSource + std::fmt::Debug + Clone> InMemoryNode<S> {
..Default::default()
};

tracing::info!("");

let bytecodes: HashMap<U256, Vec<U256>> = vm
.get_last_tx_compressed_bytecodes()
.iter()
.map(|b| bytecode_to_factory_dep(b.original.clone()))
.collect();

// vm.execute(VmExecutionMode::Bootloader);

let modified_keys = storage.borrow().modified_storage_keys().clone();

Ok((
Expand Down
2 changes: 1 addition & 1 deletion src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ pub fn mine_empty_blocks<S: std::fmt::Debug + ForkSource>(
for i in 0..num_blocks {
// roll the vm
let (keys, bytecodes, block_ctx) = {
let storage = StorageView::new(&node.fork_storage).to_rc_ptr();
let storage = StorageView::new(&node.fork_storage).into_rc_ptr();

// system_contract.contracts_for_l2_call() will give playground contracts
// we need these to use the unsafeOverrideBlock method in SystemContext.sol
Expand Down

0 comments on commit 5e7dea4

Please sign in to comment.