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: allow immutable variables to be merged during forks #719

Merged
merged 8 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
15 changes: 15 additions & 0 deletions crates/cheatcodes/src/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,21 @@ impl Cheatcodes {
}
}

// record immutable variables
if result.execution_result.is_success() {
for (addr, imm_values) in result.recorded_immutables {
let addr = addr.to_address();
let keys = imm_values
.into_keys()
.map(|slot_index| {
foundry_zksync_core::get_immutable_slot_key(addr, slot_index)
.to_ru256()
})
.collect::<HashSet<_>>();
ecx.db.save_zk_immutable_storage(addr, keys);
}
}

match result.execution_result {
ExecutionResult::Success { output, gas_used, .. } => {
let _ = gas.record_cost(gas_used);
Expand Down
9 changes: 8 additions & 1 deletion crates/evm/core/src/backend/cow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ use revm::{
},
Database, DatabaseCommit, JournaledState,
};
use std::{borrow::Cow, collections::BTreeMap};
use std::{
borrow::Cow,
collections::{BTreeMap, HashSet},
};

/// A wrapper around `Backend` that ensures only `revm::DatabaseRef` functions are called.
///
Expand Down Expand Up @@ -133,6 +136,10 @@ impl DatabaseExt for CowBackend<'_> {
self.backend.to_mut().get_fork_info(id)
}

fn save_zk_immutable_storage(&mut self, addr: Address, keys: HashSet<U256>) {
self.backend.to_mut().save_zk_immutable_storage(addr, keys)
}

fn snapshot_state(&mut self, journaled_state: &JournaledState, env: &Env) -> U256 {
self.backend_mut(env).snapshot_state(journaled_state, env)
}
Expand Down
180 changes: 122 additions & 58 deletions crates/evm/core/src/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ use eyre::Context;
use foundry_common::{is_known_system_sender, SYSTEM_TRANSACTION_TYPE};
pub use foundry_fork_db::{cache::BlockchainDbMeta, BlockchainDb, SharedBackend};
use foundry_zksync_core::{
convert::ConvertH160, ACCOUNT_CODE_STORAGE_ADDRESS, L2_BASE_TOKEN_ADDRESS, NONCE_HOLDER_ADDRESS,
convert::ConvertH160, ACCOUNT_CODE_STORAGE_ADDRESS, IMMUTABLE_SIMULATOR_STORAGE_ADDRESS,
KNOWN_CODES_STORAGE_ADDRESS, L2_BASE_TOKEN_ADDRESS, NONCE_HOLDER_ADDRESS,
};
use itertools::Itertools;
use revm::{
Expand Down Expand Up @@ -100,6 +101,14 @@ pub trait DatabaseExt: Database<Error = DatabaseError> + DatabaseCommit {
/// and the the fork environment.
fn get_fork_info(&mut self, id: LocalForkId) -> eyre::Result<ForkInfo>;

/// Saves the storage keys for immutable variables per address.
///
/// These are required during fork to help merge the persisted addresses, as they are stored
/// hashed so there is currently no way to retrieve all the address associated storage keys.
/// We store all the storage keys here, even if the addresses are not marked persistent as
/// they can be marked at a later stage as well.
fn save_zk_immutable_storage(&mut self, addr: Address, keys: HashSet<U256>);

/// Reverts the snapshot if it exists
///
/// Returns `true` if the snapshot was successfully reverted, `false` if no snapshot for that id
Expand Down Expand Up @@ -491,6 +500,8 @@ pub struct Backend {
/// The balance, nonce and code are stored under zkSync's respective system contract
/// storages. These need to be merged into the forked storage.
pub is_zk: bool,
/// Store storage keys per contract address for immutable variables.
zk_recorded_immutable_keys: HashMap<Address, HashSet<U256>>,
}

impl Backend {
Expand Down Expand Up @@ -524,6 +535,7 @@ impl Backend {
inner,
fork_url_type: Default::default(),
is_zk: false,
zk_recorded_immutable_keys: Default::default(),
};

if let Some(fork) = fork {
Expand Down Expand Up @@ -564,6 +576,7 @@ impl Backend {
inner: Default::default(),
fork_url_type: Default::default(),
is_zk: false,
zk_recorded_immutable_keys: Default::default(),
}
}

Expand Down Expand Up @@ -670,13 +683,13 @@ impl Backend {
&self,
active_journaled_state: &mut JournaledState,
target_fork: &mut Fork,
merge_zk_db: bool,
zk_state: Option<ZkMergeState>,
) {
self.update_fork_db_contracts(
self.inner.persistent_accounts.iter().copied(),
active_journaled_state,
target_fork,
merge_zk_db,
zk_state,
)
}

Expand All @@ -686,17 +699,17 @@ impl Backend {
accounts: impl IntoIterator<Item = Address>,
active_journaled_state: &mut JournaledState,
target_fork: &mut Fork,
merge_zk_db: bool,
zk_state: Option<ZkMergeState>,
) {
if let Some(db) = self.active_fork_db() {
merge_account_data(accounts, db, active_journaled_state, target_fork, merge_zk_db)
merge_account_data(accounts, db, active_journaled_state, target_fork, zk_state)
} else {
merge_account_data(
accounts,
&self.mem_db,
active_journaled_state,
target_fork,
merge_zk_db,
zk_state,
)
}
}
Expand Down Expand Up @@ -984,6 +997,13 @@ impl DatabaseExt for Backend {
Ok(ForkInfo { fork_type, fork_env })
}

fn save_zk_immutable_storage(&mut self, addr: Address, keys: HashSet<U256>) {
self.zk_recorded_immutable_keys
.entry(addr)
.and_modify(|entry| entry.extend(&keys))
.or_insert(keys);
}

fn snapshot_state(&mut self, journaled_state: &JournaledState, env: &Env) -> U256 {
trace!("create snapshot");
let id = self.inner.state_snapshots.insert(BackendStateSnapshot::new(
Expand Down Expand Up @@ -1144,6 +1164,9 @@ impl DatabaseExt for Backend {
.map(|url| self.fork_url_type.get(&url).is_zk())
.unwrap_or_default();
let merge_zk_db = is_current_zk_fork && is_target_zk_fork;
let zk_state = merge_zk_db.then(|| ZkMergeState {
persistent_immutable_keys: self.zk_recorded_immutable_keys.clone(),
});

let fork_env = self
.forks
Expand Down Expand Up @@ -1212,7 +1235,7 @@ impl DatabaseExt for Backend {
caller_account.into()
});

self.update_fork_db(active_journaled_state, &mut fork, merge_zk_db);
self.update_fork_db(active_journaled_state, &mut fork, zk_state);

// insert the fork back
self.inner.set_fork(idx, fork);
Expand Down Expand Up @@ -1930,26 +1953,33 @@ pub(crate) fn update_current_env_with_fork_env(current: &mut Env, fork: Env) {
current.tx.chain_id = fork.tx.chain_id;
}

/// Defines the zksync specific state to help during merge.
#[derive(Debug, Default)]
pub(crate) struct ZkMergeState {
persistent_immutable_keys: HashMap<Address, HashSet<U256>>,
}

/// Clones the data of the given `accounts` from the `active` database into the `fork_db`
/// This includes the data held in storage (`CacheDB`) and kept in the `JournaledState`.
pub(crate) fn merge_account_data<ExtDB: DatabaseRef>(
accounts: impl IntoIterator<Item = Address>,
active: &CacheDB<ExtDB>,
active_journaled_state: &mut JournaledState,
target_fork: &mut Fork,
merge_zk_db: bool,
zk_state: Option<ZkMergeState>,
) {
for addr in accounts.into_iter() {
merge_db_account_data(addr, active, &mut target_fork.db);
if merge_zk_db {
merge_zk_account_data(addr, active, &mut target_fork.db);
if let Some(zk_state) = &zk_state {
merge_zk_account_data(addr, active, &mut target_fork.db, zk_state);
}
merge_journaled_state_data(addr, active_journaled_state, &mut target_fork.journaled_state);
if merge_zk_db {
if let Some(zk_state) = &zk_state {
merge_zk_journaled_state_data(
addr,
active_journaled_state,
&mut target_fork.journaled_state,
zk_state,
);
}
}
Expand Down Expand Up @@ -2020,53 +2050,66 @@ fn merge_zk_account_data<ExtDB: DatabaseRef>(
addr: Address,
active: &CacheDB<ExtDB>,
fork_db: &mut ForkDB,
_zk_state: &ZkMergeState,
) {
let mut merge_system_contract_entry = |system_contract: Address, slot: U256| {
let Some(acc) = active.accounts.get(&system_contract) else { return };

// port contract cache over
if let Some(code) = active.contracts.get(&acc.info.code_hash) {
trace!("merging contract cache");
fork_db.contracts.insert(acc.info.code_hash, code.clone());
}

// prepare only the specified slot in account storage
let mut new_acc = acc.clone();
new_acc.storage = Default::default();
if let Some(value) = acc.storage.get(&slot) {
new_acc.storage.insert(slot, *value);
}
let merge_system_contract_entry =
|fork_db: &mut ForkDB, system_contract: Address, slot: U256| {
let Some(acc) = active.accounts.get(&system_contract) else { return };

// port contract cache over
if let Some(code) = active.contracts.get(&acc.info.code_hash) {
trace!("merging contract cache");
fork_db.contracts.insert(acc.info.code_hash, code.clone());
}

// port account storage over
match fork_db.accounts.entry(system_contract) {
Entry::Vacant(vacant) => {
trace!("target account not present - inserting from active");
// if the fork_db doesn't have the target account
// insert the entire thing
vacant.insert(new_acc);
// prepare only the specified slot in account storage
let mut new_acc = acc.clone();
new_acc.storage = Default::default();
if let Some(value) = acc.storage.get(&slot) {
new_acc.storage.insert(slot, *value);
}
Entry::Occupied(mut occupied) => {
trace!("target account present - merging storage slots");
// if the fork_db does have the system,
// extend the existing storage (overriding)
let fork_account = occupied.get_mut();
fork_account.storage.extend(&new_acc.storage);

// port account storage over
match fork_db.accounts.entry(system_contract) {
Entry::Vacant(vacant) => {
trace!("target account not present - inserting from active");
// if the fork_db doesn't have the target account
// insert the entire thing
vacant.insert(new_acc);
}
Entry::Occupied(mut occupied) => {
trace!("target account present - merging storage slots");
// if the fork_db does have the system,
// extend the existing storage (overriding)
let fork_account = occupied.get_mut();
fork_account.storage.extend(&new_acc.storage);
}
}
}
};
};

merge_system_contract_entry(
fork_db,
L2_BASE_TOKEN_ADDRESS.to_address(),
foundry_zksync_core::get_balance_key(addr),
);
merge_system_contract_entry(
fork_db,
ACCOUNT_CODE_STORAGE_ADDRESS.to_address(),
foundry_zksync_core::get_account_code_key(addr),
);
merge_system_contract_entry(
fork_db,
NONCE_HOLDER_ADDRESS.to_address(),
foundry_zksync_core::get_nonce_key(addr),
);

if let Some(acc) = active.accounts.get(&addr) {
merge_system_contract_entry(
fork_db,
KNOWN_CODES_STORAGE_ADDRESS.to_address(),
U256::from_be_slice(&acc.info.code_hash.0[..]),
);
}
}

/// Clones the account data from the `active_journaled_state` into the `fork_journaled_state` for
Expand All @@ -2075,40 +2118,61 @@ fn merge_zk_journaled_state_data(
addr: Address,
active_journaled_state: &JournaledState,
fork_journaled_state: &mut JournaledState,
zk_state: &ZkMergeState,
) {
let mut merge_system_contract_entry = |system_contract: Address, slot: U256| {
if let Some(acc) = active_journaled_state.state.get(&system_contract) {
// prepare only the specified slot in account storage
let mut new_acc = acc.clone();
new_acc.storage = Default::default();
if let Some(value) = acc.storage.get(&slot).cloned() {
new_acc.storage.insert(slot, value);
}

match fork_journaled_state.state.entry(system_contract) {
Entry::Vacant(vacant) => {
vacant.insert(new_acc);
let merge_system_contract_entry =
|fork_journaled_state: &mut JournaledState, system_contract: Address, slot: U256| {
if let Some(acc) = active_journaled_state.state.get(&system_contract) {
// prepare only the specified slot in account storage
let mut new_acc = acc.clone();
new_acc.storage = Default::default();
if let Some(value) = acc.storage.get(&slot).cloned() {
new_acc.storage.insert(slot, value);
}
Entry::Occupied(mut occupied) => {
let fork_account = occupied.get_mut();
fork_account.storage.extend(new_acc.storage);

match fork_journaled_state.state.entry(system_contract) {
Entry::Vacant(vacant) => {
vacant.insert(new_acc);
}
Entry::Occupied(mut occupied) => {
let fork_account = occupied.get_mut();
fork_account.storage.extend(new_acc.storage);
}
}
}
}
};
};

merge_system_contract_entry(
fork_journaled_state,
L2_BASE_TOKEN_ADDRESS.to_address(),
foundry_zksync_core::get_balance_key(addr),
);
merge_system_contract_entry(
fork_journaled_state,
ACCOUNT_CODE_STORAGE_ADDRESS.to_address(),
foundry_zksync_core::get_account_code_key(addr),
);
merge_system_contract_entry(
fork_journaled_state,
NONCE_HOLDER_ADDRESS.to_address(),
foundry_zksync_core::get_nonce_key(addr),
);

if let Some(acc) = active_journaled_state.state.get(&addr) {
merge_system_contract_entry(
fork_journaled_state,
KNOWN_CODES_STORAGE_ADDRESS.to_address(),
U256::from_be_slice(&acc.info.code_hash.0[..]),
);
}

// merge immutable storage.
let immutable_simulator_addr = IMMUTABLE_SIMULATOR_STORAGE_ADDRESS.to_address();
if let Some(immutable_storage_keys) = zk_state.persistent_immutable_keys.get(&addr) {
for slot_key in immutable_storage_keys {
merge_system_contract_entry(fork_journaled_state, immutable_simulator_addr, *slot_key);
}
}
}

/// Returns true of the address is a contract
Expand Down
8 changes: 8 additions & 0 deletions crates/forge/tests/it/zk/fork.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,11 @@ async fn test_zk_setup_fork_failure() {

TestConfig::with_filter(runner, filter).evm_spec(SpecId::SHANGHAI).run().await;
}

#[tokio::test(flavor = "multi_thread")]
async fn test_zk_immutable_vars_persist_after_fork() {
let runner = TEST_DATA_DEFAULT.runner_zksync();
let filter = Filter::new(".*", "ZkForkImmutableVarsTest", ".*");

TestConfig::with_filter(runner, filter).evm_spec(SpecId::SHANGHAI).run().await;
}
Loading