From 1873d96a583430b08946c60016625cd9bb51af07 Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Tue, 9 Apr 2024 19:27:34 +0200 Subject: [PATCH 01/23] fix(compile:zk): use factory deps from compilation --- crates/cheatcodes/src/test.rs | 1 + crates/forge/bin/cmd/create.rs | 46 ++++++++++++------------ crates/zksync/compiler/src/zksolc/mod.rs | 3 ++ 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/crates/cheatcodes/src/test.rs b/crates/cheatcodes/src/test.rs index f72f8a833..89fd72906 100644 --- a/crates/cheatcodes/src/test.rs +++ b/crates/cheatcodes/src/test.rs @@ -38,6 +38,7 @@ impl Cheatcode for zkRegisterContractCall { name: name.clone(), zk_bytecode_hash: zkBytecodeHash.0.into(), zk_deployed_bytecode: zkDeployedBytecode.clone(), + zk_factory_deps: vec![], evm_bytecode_hash: *evmBytecodeHash, evm_deployed_bytecode: evmDeployedBytecode.clone(), evm_bytecode: evmBytecode.clone(), diff --git a/crates/forge/bin/cmd/create.rs b/crates/forge/bin/cmd/create.rs index 65f343914..60f6a50d3 100644 --- a/crates/forge/bin/cmd/create.rs +++ b/crates/forge/bin/cmd/create.rs @@ -141,6 +141,7 @@ impl CreateArgs { "Could not find zksolc contract for contract {}", self.contract.name ))?; + let zk_bin = CompactBytecode { object: BytecodeObject::Bytecode(Bytes::from( contract.zk_deployed_bytecode.clone(), @@ -149,30 +150,31 @@ impl CreateArgs { source_map: Default::default(), }; - let mut factory_deps = Vec::with_capacity(self.factory_deps.len()); + info!(len = contract.zk_factory_deps.len(), extra = self.factory_deps.len(), "autodetected factory deps"); + // let mut factory_deps = vec![]; - for mut contract in std::mem::take(&mut self.factory_deps) { - if let Some(path) = contract.path.as_mut() { - *path = canonicalized(project.root().join(&path)).to_string_lossy().to_string(); - } + // for mut contract in std::mem::take(&mut self.factory_deps) { + // if let Some(path) = contract.path.as_mut() { + // *path = canonicalized(project.root().join(&path)).to_string_lossy().to_string(); + // } - let (_, bin, _) = remove_contract(&mut output, &contract).with_context(|| { - format!("Unable to find specified factory deps ({}) in project", contract.name) - })?; + // let (_, bin, _) = remove_contract(&mut output, &contract).with_context(|| { + // format!("Unable to find specified factory deps ({}) in project", contract.name) + // })?; - let zk = bin - .object - .as_bytes() - .and_then(|bytes| dual_compiled_contracts.find_evm_bytecode(&bytes.0)) - .ok_or(eyre::eyre!( - "Could not find zksolc contract for contract {}", - contract.name - ))?; + // let zk = bin + // .object + // .as_bytes() + // .and_then(|bytes| dual_compiled_contracts.find_evm_bytecode(&bytes.0)) + // .ok_or(eyre::eyre!( + // "Could not find zksolc contract for contract {}", + // contract.name + // ))?; - factory_deps.push(zk.zk_deployed_bytecode.clone()); - } + // factory_deps.push(zk.zk_deployed_bytecode.clone()); + // } - (abi, zk_bin, Some((contract, factory_deps))) + (abi, zk_bin, Some((contract, vec![]))) } else { (abi, bin, None) }; @@ -297,7 +299,7 @@ impl CreateArgs { let factory = ContractFactory::new(abi.clone(), bin.clone(), provider.clone()); let is_args_empty = args.is_empty(); - let (zk_contract, factory_deps) = match zk_data { + let (zk_contract, _factory_deps) = match zk_data { Some((zk_contract, mut factory_deps)) => { //add this contract to the list of factory deps factory_deps.push(zk_contract.zk_deployed_bytecode.clone()); @@ -308,7 +310,7 @@ impl CreateArgs { let deployer = if let Some(contract) = zk_contract { factory.deploy_tokens_zk(args.clone(), contract).context("failed to deploy contract") - .map(|deployer| deployer.set_zk_factory_deps(factory_deps.clone())) + .map(|deployer| deployer.set_zk_factory_deps(contract.zk_factory_deps.clone())) } else { factory.deploy_tokens(args.clone()).context("failed to deploy contract") }.map_err(|e| { @@ -359,7 +361,7 @@ impl CreateArgs { .set_to(NameOrAddress::from(foundry_zksync_core::CONTRACT_DEPLOYER_ADDRESS)); let estimated_gas = - foundry_zksync_core::estimate_gas(&deployer.tx, factory_deps, &provider) + foundry_zksync_core::estimate_gas(&deployer.tx, contract.zk_factory_deps.clone(), &provider) .await?; deployer.tx.set_gas(estimated_gas.limit.to_ethers()); deployer.tx.set_gas_price(estimated_gas.price.to_ethers()); diff --git a/crates/zksync/compiler/src/zksolc/mod.rs b/crates/zksync/compiler/src/zksolc/mod.rs index b07c554db..f0767f233 100644 --- a/crates/zksync/compiler/src/zksolc/mod.rs +++ b/crates/zksync/compiler/src/zksolc/mod.rs @@ -25,6 +25,8 @@ pub struct DualCompiledContract { pub zk_bytecode_hash: H256, /// Deployed bytecode hash with zksolc pub zk_deployed_bytecode: Vec, + /// Deployed bytecode factory deps + pub zk_factory_deps: Vec>, /// Deployed bytecode hash with solc pub evm_bytecode_hash: B256, /// Deployed bytecode with solc @@ -68,6 +70,7 @@ pub fn new_dual_compiled_contracts( name: contract_name, zk_bytecode_hash: packed_bytecode.bytecode_hash(), zk_deployed_bytecode: packed_bytecode.bytecode(), + zk_factory_deps: packed_bytecode.factory_deps(), evm_bytecode_hash: keccak256(solc_deployed_bytecode), evm_bytecode: solc_bytecode.to_vec(), evm_deployed_bytecode: solc_deployed_bytecode.to_vec(), From e7a1264995f5402145e68a5beb18298034711105 Mon Sep 17 00:00:00 2001 From: Jrigada Date: Tue, 9 Apr 2024 17:05:06 -0300 Subject: [PATCH 02/23] algo --- crates/forge/bin/cmd/create.rs | 101 +++++++++++++++++------ crates/zksync/compiler/src/zksolc/mod.rs | 7 ++ 2 files changed, 83 insertions(+), 25 deletions(-) diff --git a/crates/forge/bin/cmd/create.rs b/crates/forge/bin/cmd/create.rs index 60f6a50d3..5968a33ac 100644 --- a/crates/forge/bin/cmd/create.rs +++ b/crates/forge/bin/cmd/create.rs @@ -14,6 +14,7 @@ use ethers_core::{ use ethers_middleware::SignerMiddleware; use ethers_providers::Middleware; use eyre::{Context, Result}; +use forge::revm::primitives::bitvec::vec; use foundry_cli::{ opts::{CoreBuildArgs, EthereumOpts, EtherscanOpts, TransactionOpts}, utils::{self, read_constructor_args_file, remove_contract, LoadConfig}, @@ -25,7 +26,7 @@ use foundry_common::{ types::{ToAlloy, ToEthers}, }; use foundry_compilers::{ - artifacts::{BytecodeObject, CompactBytecode}, + artifacts::{contract, BytecodeObject, CompactBytecode}, info::ContractInfo, utils::canonicalized, }; @@ -34,7 +35,13 @@ use foundry_zksync_compiler::{ new_dual_compiled_contracts, DualCompiledContract, FindContract, ZkSolc, }; use serde_json::json; -use std::{borrow::Borrow, marker::PhantomData, path::PathBuf, sync::Arc}; +use std::{ + borrow::Borrow, + collections::{HashSet, VecDeque}, + marker::PhantomData, + path::PathBuf, + sync::Arc, +}; /// CLI arguments for `forge create`. #[derive(Clone, Debug, Parser)] @@ -150,31 +157,38 @@ impl CreateArgs { source_map: Default::default(), }; - info!(len = contract.zk_factory_deps.len(), extra = self.factory_deps.len(), "autodetected factory deps"); - // let mut factory_deps = vec![]; + let found_factory_deps = + fetch_all_factory_deps(dual_compiled_contracts.clone(), &contract.zk_factory_deps); + + println!("Factory Deps: {:?}", found_factory_deps.len()); + let mut factory_deps = vec![]; + + for mut contract in std::mem::take(&mut self.factory_deps) { + if let Some(path) = contract.path.as_mut() { + *path = canonicalized(project.root().join(&path)).to_string_lossy().to_string(); + } - // for mut contract in std::mem::take(&mut self.factory_deps) { - // if let Some(path) = contract.path.as_mut() { - // *path = canonicalized(project.root().join(&path)).to_string_lossy().to_string(); - // } + let (_, bin, _) = remove_contract(&mut output, &contract).with_context(|| { + format!("Unable to find specified factory deps ({}) in project", contract.name) + })?; - // let (_, bin, _) = remove_contract(&mut output, &contract).with_context(|| { - // format!("Unable to find specified factory deps ({}) in project", contract.name) - // })?; + let zk = bin + .object + .as_bytes() + .and_then(|bytes| dual_compiled_contracts.find_evm_bytecode(&bytes.0)) + .ok_or(eyre::eyre!( + "Could not find zksolc contract for contract {}", + contract.name + ))?; - // let zk = bin - // .object - // .as_bytes() - // .and_then(|bytes| dual_compiled_contracts.find_evm_bytecode(&bytes.0)) - // .ok_or(eyre::eyre!( - // "Could not find zksolc contract for contract {}", - // contract.name - // ))?; + println!("factory dep bytecode {:?}", zk.zk_deployed_bytecode.clone()); + + factory_deps.push(zk.zk_deployed_bytecode.clone()); + } - // factory_deps.push(zk.zk_deployed_bytecode.clone()); - // } + factory_deps.extend(found_factory_deps); - (abi, zk_bin, Some((contract, vec![]))) + (abi, zk_bin, Some((contract, factory_deps))) } else { (abi, bin, None) }; @@ -360,9 +374,12 @@ impl CreateArgs { .tx .set_to(NameOrAddress::from(foundry_zksync_core::CONTRACT_DEPLOYER_ADDRESS)); - let estimated_gas = - foundry_zksync_core::estimate_gas(&deployer.tx, contract.zk_factory_deps.clone(), &provider) - .await?; + let estimated_gas = foundry_zksync_core::estimate_gas( + &deployer.tx, + contract.zk_factory_deps.clone(), + &provider, + ) + .await?; deployer.tx.set_gas(estimated_gas.limit.to_ethers()); deployer.tx.set_gas_price(estimated_gas.price.to_ethers()); } @@ -847,3 +864,37 @@ mod tests { let _params = args.parse_constructor_args(&constructor, &args.constructor_args).unwrap(); } } + +fn fetch_all_factory_deps( + dual_compiled_contracts: Vec, + initial_deps: &[Vec], +) -> Vec> { + let mut visited = HashSet::new(); + let mut queue = VecDeque::new(); + let mut all_factory_deps = Vec::new(); + + for dep in initial_deps.iter().cloned() { + queue.push_back(dep); + } + + while let Some(dep) = queue.pop_front() { + if visited.insert(dep.clone()) { + if let Some(contract) = dual_compiled_contracts.find_zk_deployed_bytecode(&dep) { + + + println!("Processing contract: {:?}", contract.name); + println!("Factory Deps: {:?}", contract.zk_factory_deps.len()); + for nested_dep in &contract.zk_factory_deps { + println!("Nested Dep: {:?}", nested_dep); + if visited.insert(nested_dep.clone()) { + queue.push_back(nested_dep.clone()); + } + } + + all_factory_deps.push(dep); + } + } + } + + all_factory_deps +} diff --git a/crates/zksync/compiler/src/zksolc/mod.rs b/crates/zksync/compiler/src/zksolc/mod.rs index f0767f233..737432bef 100644 --- a/crates/zksync/compiler/src/zksolc/mod.rs +++ b/crates/zksync/compiler/src/zksolc/mod.rs @@ -89,9 +89,16 @@ pub trait FindContract { /// Finds a contract matching the ZK bytecode hash fn find_zk_bytecode_hash(&self, code_hash: H256) -> Option<&DualCompiledContract>; + + /// Finds a contract matching the ZK deployed bytecode + fn find_zk_deployed_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract>; } impl FindContract for Vec { + fn find_zk_deployed_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract> { + self.iter().find(|contract| bytecode.starts_with(&contract.zk_deployed_bytecode)) + } + fn find_evm_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract> { self.iter().find(|contract| bytecode.starts_with(&contract.evm_bytecode)) } From 8631f6c6256e838bed44a2a1c91fd7ba89b0f5a9 Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Tue, 9 Apr 2024 23:54:22 +0200 Subject: [PATCH 03/23] feat(create:zk): fetch all unique factory deps chore: impl FetchContract for &[DualCompiledContract] refactor(create:zk): avoid modifying provided list of factory_deps during deploy fix(create:zk): simplify factory dep search algo --- crates/forge/bin/cmd/create.rs | 75 ++++++------------------ crates/zksync/compiler/src/zksolc/mod.rs | 14 +++++ 2 files changed, 32 insertions(+), 57 deletions(-) diff --git a/crates/forge/bin/cmd/create.rs b/crates/forge/bin/cmd/create.rs index 5968a33ac..d7c519938 100644 --- a/crates/forge/bin/cmd/create.rs +++ b/crates/forge/bin/cmd/create.rs @@ -157,37 +157,10 @@ impl CreateArgs { source_map: Default::default(), }; - let found_factory_deps = - fetch_all_factory_deps(dual_compiled_contracts.clone(), &contract.zk_factory_deps); - - println!("Factory Deps: {:?}", found_factory_deps.len()); - let mut factory_deps = vec![]; - - for mut contract in std::mem::take(&mut self.factory_deps) { - if let Some(path) = contract.path.as_mut() { - *path = canonicalized(project.root().join(&path)).to_string_lossy().to_string(); - } - - let (_, bin, _) = remove_contract(&mut output, &contract).with_context(|| { - format!("Unable to find specified factory deps ({}) in project", contract.name) - })?; - - let zk = bin - .object - .as_bytes() - .and_then(|bytes| dual_compiled_contracts.find_evm_bytecode(&bytes.0)) - .ok_or(eyre::eyre!( - "Could not find zksolc contract for contract {}", - contract.name - ))?; - - println!("factory dep bytecode {:?}", zk.zk_deployed_bytecode.clone()); - - factory_deps.push(zk.zk_deployed_bytecode.clone()); - } - - factory_deps.extend(found_factory_deps); + let factory_deps = + fetch_all_factory_deps(&dual_compiled_contracts, &contract).into_iter().collect::>(); + println!("Total Factory Deps: {:?}", factory_deps.len()); (abi, zk_bin, Some((contract, factory_deps))) } else { (abi, bin, None) @@ -313,18 +286,9 @@ impl CreateArgs { let factory = ContractFactory::new(abi.clone(), bin.clone(), provider.clone()); let is_args_empty = args.is_empty(); - let (zk_contract, _factory_deps) = match zk_data { - Some((zk_contract, mut factory_deps)) => { - //add this contract to the list of factory deps - factory_deps.push(zk_contract.zk_deployed_bytecode.clone()); - (Some(zk_contract), factory_deps) - } - None => (None, vec![]), - }; - - let deployer = if let Some(contract) = zk_contract { + let deployer = if let Some((contract, factory_deps)) = &zk_data { factory.deploy_tokens_zk(args.clone(), contract).context("failed to deploy contract") - .map(|deployer| deployer.set_zk_factory_deps(contract.zk_factory_deps.clone())) + .map(|deployer| deployer.set_zk_factory_deps(factory_deps.clone())) } else { factory.deploy_tokens(args.clone()).context("failed to deploy contract") }.map_err(|e| { @@ -344,9 +308,9 @@ impl CreateArgs { deployer.tx.set_value(value.to_ethers()); } - match zk_contract { + match zk_data { None => provider.fill_transaction(&mut deployer.tx, None).await?, - Some(contract) => { + Some((contract, factory_deps)) => { let chain_id = provider.get_chainid().await?.as_u64(); deployer.tx.set_chain_id(chain_id); @@ -376,7 +340,7 @@ impl CreateArgs { let estimated_gas = foundry_zksync_core::estimate_gas( &deployer.tx, - contract.zk_factory_deps.clone(), + factory_deps.clone(), &provider, ) .await?; @@ -866,35 +830,32 @@ mod tests { } fn fetch_all_factory_deps( - dual_compiled_contracts: Vec, - initial_deps: &[Vec], -) -> Vec> { + dual_compiled_contracts: &[DualCompiledContract], + root: &DualCompiledContract, +) -> HashSet> { let mut visited = HashSet::new(); let mut queue = VecDeque::new(); - let mut all_factory_deps = Vec::new(); - for dep in initial_deps.iter().cloned() { + for dep in root.zk_factory_deps.iter().cloned() { queue.push_back(dep); } while let Some(dep) = queue.pop_front() { + //try to insert in the list of visited, if it's already present, skip if visited.insert(dep.clone()) { if let Some(contract) = dual_compiled_contracts.find_zk_deployed_bytecode(&dep) { + debug!(name = contract.name, deps = contract.zk_factory_deps.len(), "new factory depdendency"); - - println!("Processing contract: {:?}", contract.name); - println!("Factory Deps: {:?}", contract.zk_factory_deps.len()); for nested_dep in &contract.zk_factory_deps { - println!("Nested Dep: {:?}", nested_dep); - if visited.insert(nested_dep.clone()) { + //check that the nested dependency is inserted + if !visited.contains(nested_dep) { + //if not, add it to queue for processing queue.push_back(nested_dep.clone()); } } - - all_factory_deps.push(dep); } } } - all_factory_deps + visited } diff --git a/crates/zksync/compiler/src/zksolc/mod.rs b/crates/zksync/compiler/src/zksolc/mod.rs index 737432bef..a18df1025 100644 --- a/crates/zksync/compiler/src/zksolc/mod.rs +++ b/crates/zksync/compiler/src/zksolc/mod.rs @@ -107,3 +107,17 @@ impl FindContract for Vec { self.iter().find(|contract| code_hash == contract.zk_bytecode_hash) } } + +impl FindContract for &[DualCompiledContract] { + fn find_zk_deployed_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract> { + self.iter().find(|contract| bytecode.starts_with(&contract.zk_deployed_bytecode)) + } + + fn find_evm_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract> { + self.iter().find(|contract| bytecode.starts_with(&contract.evm_bytecode)) + } + + fn find_zk_bytecode_hash(&self, code_hash: H256) -> Option<&DualCompiledContract> { + self.iter().find(|contract| code_hash == contract.zk_bytecode_hash) + } +} From 4ce160c2caccec7dd7fea843aea507c886582d3f Mon Sep 17 00:00:00 2001 From: Jrigada Date: Wed, 10 Apr 2024 11:18:09 -0300 Subject: [PATCH 04/23] move fetch factory deps to FindContract trait --- crates/forge/bin/cmd/create.rs | 37 ++----------- crates/zksync/compiler/src/zksolc/mod.rs | 70 +++++++++++++++++++++++- 2 files changed, 73 insertions(+), 34 deletions(-) diff --git a/crates/forge/bin/cmd/create.rs b/crates/forge/bin/cmd/create.rs index d7c519938..f8c5dc0a1 100644 --- a/crates/forge/bin/cmd/create.rs +++ b/crates/forge/bin/cmd/create.rs @@ -157,8 +157,10 @@ impl CreateArgs { source_map: Default::default(), }; - let factory_deps = - fetch_all_factory_deps(&dual_compiled_contracts, &contract).into_iter().collect::>(); + let factory_deps = dual_compiled_contracts + .fetch_all_factory_deps(&contract) + .into_iter() + .collect::>(); println!("Total Factory Deps: {:?}", factory_deps.len()); (abi, zk_bin, Some((contract, factory_deps))) @@ -828,34 +830,3 @@ mod tests { let _params = args.parse_constructor_args(&constructor, &args.constructor_args).unwrap(); } } - -fn fetch_all_factory_deps( - dual_compiled_contracts: &[DualCompiledContract], - root: &DualCompiledContract, -) -> HashSet> { - let mut visited = HashSet::new(); - let mut queue = VecDeque::new(); - - for dep in root.zk_factory_deps.iter().cloned() { - queue.push_back(dep); - } - - while let Some(dep) = queue.pop_front() { - //try to insert in the list of visited, if it's already present, skip - if visited.insert(dep.clone()) { - if let Some(contract) = dual_compiled_contracts.find_zk_deployed_bytecode(&dep) { - debug!(name = contract.name, deps = contract.zk_factory_deps.len(), "new factory depdendency"); - - for nested_dep in &contract.zk_factory_deps { - //check that the nested dependency is inserted - if !visited.contains(nested_dep) { - //if not, add it to queue for processing - queue.push_back(nested_dep.clone()); - } - } - } - } - } - - visited -} diff --git a/crates/zksync/compiler/src/zksolc/mod.rs b/crates/zksync/compiler/src/zksolc/mod.rs index a18df1025..b66baecfb 100644 --- a/crates/zksync/compiler/src/zksolc/mod.rs +++ b/crates/zksync/compiler/src/zksolc/mod.rs @@ -5,7 +5,7 @@ mod config; mod factory_deps; mod manager; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet, VecDeque}; pub use compile::*; pub use config::*; @@ -14,6 +14,7 @@ use foundry_compilers::{Artifact, ProjectCompileOutput}; pub use manager::*; use alloy_primitives::{keccak256, B256}; +use tracing::debug; use zksync_types::H256; /// Defines a contract that has been dual compiled with both zksolc and solc @@ -92,6 +93,9 @@ pub trait FindContract { /// Finds a contract matching the ZK deployed bytecode fn find_zk_deployed_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract>; + + /// Finds a contract own and nested factory deps + fn fetch_all_factory_deps(&self, root: &DualCompiledContract) -> HashSet>; } impl FindContract for Vec { @@ -106,6 +110,38 @@ impl FindContract for Vec { fn find_zk_bytecode_hash(&self, code_hash: H256) -> Option<&DualCompiledContract> { self.iter().find(|contract| code_hash == contract.zk_bytecode_hash) } + + fn fetch_all_factory_deps(&self, root: &DualCompiledContract) -> HashSet> { + let mut visited = HashSet::new(); + let mut queue = VecDeque::new(); + + for dep in root.zk_factory_deps.iter().cloned() { + queue.push_back(dep); + } + + while let Some(dep) = queue.pop_front() { + //try to insert in the list of visited, if it's already present, skip + if visited.insert(dep.clone()) { + if let Some(contract) = self.find_zk_deployed_bytecode(&dep) { + debug!( + name = contract.name, + deps = contract.zk_factory_deps.len(), + "new factory depdendency" + ); + + for nested_dep in &contract.zk_factory_deps { + //check that the nested dependency is inserted + if !visited.contains(nested_dep) { + //if not, add it to queue for processing + queue.push_back(nested_dep.clone()); + } + } + } + } + } + + visited + } } impl FindContract for &[DualCompiledContract] { @@ -120,4 +156,36 @@ impl FindContract for &[DualCompiledContract] { fn find_zk_bytecode_hash(&self, code_hash: H256) -> Option<&DualCompiledContract> { self.iter().find(|contract| code_hash == contract.zk_bytecode_hash) } + + fn fetch_all_factory_deps(&self, root: &DualCompiledContract) -> HashSet> { + let mut visited = HashSet::new(); + let mut queue = VecDeque::new(); + + for dep in root.zk_factory_deps.iter().cloned() { + queue.push_back(dep); + } + + while let Some(dep) = queue.pop_front() { + //try to insert in the list of visited, if it's already present, skip + if visited.insert(dep.clone()) { + if let Some(contract) = self.find_zk_deployed_bytecode(&dep) { + debug!( + name = contract.name, + deps = contract.zk_factory_deps.len(), + "new factory depdendency" + ); + + for nested_dep in &contract.zk_factory_deps { + //check that the nested dependency is inserted + if !visited.contains(nested_dep) { + //if not, add it to queue for processing + queue.push_back(nested_dep.clone()); + } + } + } + } + } + + visited + } } From 75f82b8ce23d6e2353281b8165fcca323bb5c4d0 Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Wed, 10 Apr 2024 16:57:27 +0200 Subject: [PATCH 05/23] fix(zkvm:broadcast): fetch all factory_deps --- crates/cheatcodes/src/inspector.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/cheatcodes/src/inspector.rs b/crates/cheatcodes/src/inspector.rs index fc4c47d43..5e9454454 100644 --- a/crates/cheatcodes/src/inspector.rs +++ b/crates/cheatcodes/src/inspector.rs @@ -1573,6 +1573,12 @@ impl Inspector for Cheatcodes { .unwrap_or_else(|| { panic!("failed finding contract for {:?}", call.init_code) }); + let factory_deps = self + .dual_compiled_contracts + .fetch_all_factory_deps(contract) + .into_iter() + .collect(); + let constructor_input = call.init_code[contract.evm_bytecode.len()..].to_vec(); let create_input = foundry_zksync_core::encode_create_params( @@ -1581,7 +1587,6 @@ impl Inspector for Cheatcodes { constructor_input, ); bytecode = Bytes::from(create_input); - let factory_deps = vec![contract.zk_deployed_bytecode.clone()]; Some(ZkTransactionMetadata { factory_deps }) } else { From 36f32f73f5acaecaf4ea90e8b0ceaa2b4cef7935 Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Wed, 10 Apr 2024 21:51:55 +0200 Subject: [PATCH 06/23] fix(zkvm:create): fetch all factory deps --- crates/cheatcodes/src/inspector.rs | 1 + crates/zksync/core/src/vm/runner.rs | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/cheatcodes/src/inspector.rs b/crates/cheatcodes/src/inspector.rs index 5e9454454..310ab7a30 100644 --- a/crates/cheatcodes/src/inspector.rs +++ b/crates/cheatcodes/src/inspector.rs @@ -1690,6 +1690,7 @@ impl Inspector for Cheatcodes { if let Ok(result) = foundry_zksync_core::vm::create::<_, DatabaseError>( call, zk_contract, + &self.dual_compiled_contracts, data.env, data.db, &mut data.journaled_state, diff --git a/crates/zksync/core/src/vm/runner.rs b/crates/zksync/core/src/vm/runner.rs index 06b4ded44..a58e3df90 100644 --- a/crates/zksync/core/src/vm/runner.rs +++ b/crates/zksync/core/src/vm/runner.rs @@ -16,7 +16,7 @@ use foundry_common::{ console::HARDHAT_CONSOLE_ADDRESS, fmt::ConsoleFmt, patch_hh_console_selector, Console, HardhatConsole, }; -use foundry_zksync_compiler::DualCompiledContract; +use foundry_zksync_compiler::{DualCompiledContract, FindContract}; use std::{collections::HashMap, fmt::Debug, str::FromStr, sync::Arc}; use crate::{fix_l2_gas_limit, fix_l2_gas_price}; @@ -164,6 +164,7 @@ where pub fn create<'a, DB, E>( call: &CreateInputs, contract: &DualCompiledContract, + dual_compiled_contracts: &[DualCompiledContract], env: &'a mut Env, db: &'a mut DB, journaled_state: &'a mut JournaledState, @@ -177,7 +178,8 @@ where let constructor_input = call.init_code[contract.evm_bytecode.len()..].to_vec(); let caller = env.tx.caller; let calldata = encode_create_params(&call.scheme, contract.zk_bytecode_hash, constructor_input); - let factory_deps = vec![contract.zk_deployed_bytecode.clone()]; + let factory_deps = + dual_compiled_contracts.fetch_all_factory_deps(&contract).into_iter().collect(); let nonce = ZKVMData::new(db, journaled_state).get_tx_nonce(caller); let (gas_limit, max_fee_per_gas) = gas_params(env, db, journaled_state, caller); From 96bcb0e7528c0244f49c2c304451efcb4ed00529 Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Thu, 11 Apr 2024 19:23:18 +0200 Subject: [PATCH 07/23] chore: cleanup & todos --- crates/cheatcodes/src/test.rs | 1 + crates/forge/bin/cmd/create.rs | 9 ++++----- crates/forge/tests/it/config.rs | 1 + crates/zksync/core/src/vm/runner.rs | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/cheatcodes/src/test.rs b/crates/cheatcodes/src/test.rs index 89fd72906..69d52126e 100644 --- a/crates/cheatcodes/src/test.rs +++ b/crates/cheatcodes/src/test.rs @@ -38,6 +38,7 @@ impl Cheatcode for zkRegisterContractCall { name: name.clone(), zk_bytecode_hash: zkBytecodeHash.0.into(), zk_deployed_bytecode: zkDeployedBytecode.clone(), + //TODO: add argument to cheatcode zk_factory_deps: vec![], evm_bytecode_hash: *evmBytecodeHash, evm_deployed_bytecode: evmDeployedBytecode.clone(), diff --git a/crates/forge/bin/cmd/create.rs b/crates/forge/bin/cmd/create.rs index f8c5dc0a1..dbbe05b13 100644 --- a/crates/forge/bin/cmd/create.rs +++ b/crates/forge/bin/cmd/create.rs @@ -14,7 +14,6 @@ use ethers_core::{ use ethers_middleware::SignerMiddleware; use ethers_providers::Middleware; use eyre::{Context, Result}; -use forge::revm::primitives::bitvec::vec; use foundry_cli::{ opts::{CoreBuildArgs, EthereumOpts, EtherscanOpts, TransactionOpts}, utils::{self, read_constructor_args_file, remove_contract, LoadConfig}, @@ -26,7 +25,7 @@ use foundry_common::{ types::{ToAlloy, ToEthers}, }; use foundry_compilers::{ - artifacts::{contract, BytecodeObject, CompactBytecode}, + artifacts::{BytecodeObject, CompactBytecode}, info::ContractInfo, utils::canonicalized, }; @@ -37,7 +36,6 @@ use foundry_zksync_compiler::{ use serde_json::json; use std::{ borrow::Borrow, - collections::{HashSet, VecDeque}, marker::PhantomData, path::PathBuf, sync::Arc, @@ -157,12 +155,13 @@ impl CreateArgs { source_map: Default::default(), }; + //TODO: restore `--factory-deps` handling? (+ lookup) + // or remove flag entirely? let factory_deps = dual_compiled_contracts - .fetch_all_factory_deps(&contract) + .fetch_all_factory_deps(contract) .into_iter() .collect::>(); - println!("Total Factory Deps: {:?}", factory_deps.len()); (abi, zk_bin, Some((contract, factory_deps))) } else { (abi, bin, None) diff --git a/crates/forge/tests/it/config.rs b/crates/forge/tests/it/config.rs index 342b0d462..a18fdfae6 100644 --- a/crates/forge/tests/it/config.rs +++ b/crates/forge/tests/it/config.rs @@ -240,6 +240,7 @@ pub async fn runner_with_config_and_zk(mut config: Config) -> MultiContractRunne name: contract_name, zk_bytecode_hash: packed_bytecode.bytecode_hash(), zk_deployed_bytecode: packed_bytecode.bytecode(), + zk_factory_deps: packed_bytecode.factory_deps(), evm_bytecode_hash: keccak256(solc_deployed_bytecode), evm_bytecode: solc_bytecode.to_vec(), evm_deployed_bytecode: solc_deployed_bytecode.to_vec(), diff --git a/crates/zksync/core/src/vm/runner.rs b/crates/zksync/core/src/vm/runner.rs index a58e3df90..9e4d46ef5 100644 --- a/crates/zksync/core/src/vm/runner.rs +++ b/crates/zksync/core/src/vm/runner.rs @@ -179,7 +179,7 @@ where let caller = env.tx.caller; let calldata = encode_create_params(&call.scheme, contract.zk_bytecode_hash, constructor_input); let factory_deps = - dual_compiled_contracts.fetch_all_factory_deps(&contract).into_iter().collect(); + dual_compiled_contracts.fetch_all_factory_deps(contract).into_iter().collect(); let nonce = ZKVMData::new(db, journaled_state).get_tx_nonce(caller); let (gas_limit, max_fee_per_gas) = gas_params(env, db, journaled_state, caller); From 44a5778d19641f301bdfb6d3099fece13c4cc0dc Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Thu, 11 Apr 2024 20:02:04 +0200 Subject: [PATCH 08/23] tests(zk): add `Factory` family of tests --- zk-tests/src/Factory.t.sol | 131 +++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 zk-tests/src/Factory.t.sol diff --git a/zk-tests/src/Factory.t.sol b/zk-tests/src/Factory.t.sol new file mode 100644 index 000000000..93d1854ef --- /dev/null +++ b/zk-tests/src/Factory.t.sol @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity ^0.8.0; + +import {Test} from 'forge-std/Test.sol'; + +/// Set of tests for factory contracts +/// +/// *Constructor factories build their dependencies in their constructors +/// *User factories don't deploy but assume the given address to be a deployed factory + +contract MyContract { + uint256 public number; + constructor(uint256 _number) { + number = _number; + } +} + +contract MyClassicFactory { + MyContract item; + + function create(uint256 _number) public { + item = new MyContract(_number); + } + + function getNumber() public view returns (uint256) { + return item.number(); + } +} + +contract MyConstructorFactory { + MyContract item; + + constructor(uint256 _number) { + item = new MyContract(_number); + } + + function getNumber() public view returns (uint256) { + return item.number(); + } +} + +contract MyNestedFactory { + MyClassicFactory nested; + + function create(uint256 _number) { + nested = new MyClassicFactory(); + + nested.create(_number); + } + + function getNumber() public view returns (uint256) { + return nested.getNumber(); + } +} + +contract MyNestedConstructorFactory { + MyClassicFactory nested; + + constructor(uint256 _number) { + nested = new MyClassicFactory(); + + nested.create(_number); + } + + function getNumber() public view returns (uint256) { + return nested.getNumber(); + } +} + +contract MyUserFactory { + function create(address classicFactory, uint256 _number) public { + MyClassicFactory(classicFactory).create(_number); + } + + function getNumber(address classicFactory) public returns (uint256) { + return MyClassicFactory(classicFactory).getNumber(); + } +} + +contract MyUserConstructorFactory { + constructor(address classicFactory, uint256 _number) { + MyClassicFactory(classicFactory).create(_number); + } + + function getNumber(address classicFactory) public returns (uint256) { + return MyClassicFactory(classicFactory).getNumber(); + } +} + +contract ZkFactory is Test { + function testClassicFactory() public { + MyClassicFactory factory = new MyClassicFactory(); + factory.create(42); + + assert(factory.getNumber() == 42); + } + + function testConstructorFactory() public { + MyConstructorFactory factory = new MyConstructorFactory(42); + + assert(factory.getNumber() == 42); + } + + function testNestedFactory() public { + MyNestedFactory factory = new MyNestedFactory(); + factory.create(42); + + assert(factory.getNumber() == 42); + } + + function testNestedConstructorFactory() public { + MyNestedConstructorFactory factory = new MyNestedConstructorFactory(42); + + assert(factory.getNumber() == 42); + } + + function testUserFactory() public { + MyClassicFactory factory = new MyClassicFactory(); + MyUserFactory user = new MyUserFactory(address(factory)); + user.create(42); + + assert(user.getNumber() == 42); + } + + function testUserConstructorFactory() public { + MyClassicFactory factory = new MyClassicFactory(); + MyUserConstructorFactory factory = new MyUserConstructorFactory(address(factory), 42); + + assert(factory.getNumber() == 42); + } +} From 9d44a956842c24f3bed29aeaabc586bd142dd2de Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Fri, 12 Apr 2024 17:37:51 +0200 Subject: [PATCH 09/23] refactor: dedicated `DualCompiledContracts` --- crates/cheatcodes/src/config.rs | 6 +- crates/cheatcodes/src/inspector.rs | 24 +-- crates/forge/bin/cmd/create.rs | 18 +-- crates/forge/bin/cmd/script/broadcast.rs | 8 +- crates/forge/bin/cmd/script/build.rs | 6 +- crates/forge/bin/cmd/script/cmd.rs | 6 +- crates/forge/bin/cmd/script/executor.rs | 10 +- crates/forge/bin/cmd/test/mod.rs | 4 +- crates/forge/tests/it/config.rs | 4 +- crates/zksync/compiler/src/zksolc/mod.rs | 179 +++++++++-------------- crates/zksync/core/src/vm/runner.rs | 6 +- 11 files changed, 113 insertions(+), 158 deletions(-) diff --git a/crates/cheatcodes/src/config.rs b/crates/cheatcodes/src/config.rs index f44ce1652..7690ee5fd 100644 --- a/crates/cheatcodes/src/config.rs +++ b/crates/cheatcodes/src/config.rs @@ -8,7 +8,7 @@ use foundry_config::{ ResolvedRpcEndpoints, }; use foundry_evm_core::opts::EvmOpts; -use foundry_zksync_compiler::DualCompiledContract; +use foundry_zksync_compiler::DualCompiledContracts; use std::{ collections::HashMap, path::{Path, PathBuf}, @@ -42,7 +42,7 @@ pub struct CheatsConfig { /// Script wallets pub script_wallets: Option, /// ZKSolc -> Solc Contract codes - pub dual_compiled_contracts: Vec, + pub dual_compiled_contracts: DualCompiledContracts, /// Use ZK-VM on startup pub use_zk: bool, } @@ -53,7 +53,7 @@ impl CheatsConfig { config: &Config, evm_opts: EvmOpts, script_wallets: Option, - dual_compiled_contracts: Vec, + dual_compiled_contracts: DualCompiledContracts, use_zk: bool, ) -> Self { let mut allowed_paths = vec![config.__root.0.clone()]; diff --git a/crates/cheatcodes/src/inspector.rs b/crates/cheatcodes/src/inspector.rs index 310ab7a30..389ff241e 100644 --- a/crates/cheatcodes/src/inspector.rs +++ b/crates/cheatcodes/src/inspector.rs @@ -28,7 +28,7 @@ use foundry_evm_core::{ HARDHAT_CONSOLE_ADDRESS, }, }; -use foundry_zksync_compiler::{DualCompiledContract, FindContract}; +use foundry_zksync_compiler::DualCompiledContracts; use foundry_zksync_core::{ convert::{ConvertAddress, ConvertH160, ConvertH256, ConvertRU256, ConvertU256}, ZkTransactionMetadata, @@ -231,7 +231,7 @@ pub struct Cheatcodes { pub use_zk_vm: bool, /// Dual compiled contracts - pub dual_compiled_contracts: Vec, + pub dual_compiled_contracts: DualCompiledContracts, /// Logs printed during ZK-VM execution. /// EVM logs have the value `None` so they can be interpolated later, since @@ -404,10 +404,8 @@ impl Cheatcodes { .map(|(value, _)| value) .ok() .and_then(|zk_bytecode_hash| { - let zk_bytecode_hash = zk_bytecode_hash.to_h256(); self.dual_compiled_contracts - .iter() - .find(|c| c.zk_bytecode_hash == zk_bytecode_hash) + .find_by_zk_bytecode_hash(zk_bytecode_hash.to_h256()) .map(|contract| { ( contract.evm_bytecode_hash, @@ -1197,7 +1195,7 @@ impl Inspector for Cheatcodes { .unwrap_or_default(); let contract = if code_hash != KECCAK_EMPTY { self.dual_compiled_contracts - .find_zk_bytecode_hash(zksync_types::H256::from(code_hash.0)) + .find_by_zk_bytecode_hash(zksync_types::H256::from(code_hash.0)) } else { None }; @@ -1569,7 +1567,7 @@ impl Inspector for Cheatcodes { ) as u64; let contract = self .dual_compiled_contracts - .find_evm_bytecode(&call.init_code.0) + .find_by_evm_bytecode(&call.init_code.0) .unwrap_or_else(|| { panic!("failed finding contract for {:?}", call.init_code) }); @@ -1577,6 +1575,7 @@ impl Inspector for Cheatcodes { .dual_compiled_contracts .fetch_all_factory_deps(contract) .into_iter() + .map(|bc| bc.to_vec()) .collect(); let constructor_input = @@ -1679,9 +1678,16 @@ impl Inspector for Cheatcodes { let zk_contract = self .dual_compiled_contracts - .find_evm_bytecode(&call.init_code.0) + .find_by_evm_bytecode(&call.init_code.0) .unwrap_or_else(|| panic!("failed finding contract for {:?}", call.init_code)); + let factory_deps = self + .dual_compiled_contracts + .fetch_all_factory_deps(zk_contract) + .into_iter() + .map(|bc| bc.to_vec()) + .collect(); + tracing::debug!(contract = zk_contract.name, "using dual compiled contract"); let ccx = foundry_zksync_core::vm::CheatcodeTracerContext { mocked_calls: self.mocked_calls.clone(), @@ -1690,7 +1696,7 @@ impl Inspector for Cheatcodes { if let Ok(result) = foundry_zksync_core::vm::create::<_, DatabaseError>( call, zk_contract, - &self.dual_compiled_contracts, + factory_deps, data.env, data.db, &mut data.journaled_state, diff --git a/crates/forge/bin/cmd/create.rs b/crates/forge/bin/cmd/create.rs index dbbe05b13..044907305 100644 --- a/crates/forge/bin/cmd/create.rs +++ b/crates/forge/bin/cmd/create.rs @@ -30,16 +30,9 @@ use foundry_compilers::{ utils::canonicalized, }; use foundry_wallets::WalletSigner; -use foundry_zksync_compiler::{ - new_dual_compiled_contracts, DualCompiledContract, FindContract, ZkSolc, -}; +use foundry_zksync_compiler::{DualCompiledContract, DualCompiledContracts, ZkSolc}; use serde_json::json; -use std::{ - borrow::Borrow, - marker::PhantomData, - path::PathBuf, - sync::Arc, -}; +use std::{borrow::Borrow, marker::PhantomData, path::PathBuf, sync::Arc}; /// CLI arguments for `forge create`. #[derive(Clone, Debug, Parser)] @@ -128,7 +121,7 @@ impl CreateArgs { Ok(compiled) => compiled, Err(e) => return Err(eyre::eyre!("Failed to compile with zksolc: {}", e)), }; - let dual_compiled_contracts = new_dual_compiled_contracts(&output, &zk_output); + let dual_compiled_contracts = DualCompiledContracts::new(&output, &zk_output); if let Some(ref mut path) = self.contract.path { // paths are absolute in the project's output @@ -141,7 +134,7 @@ impl CreateArgs { let contract = bin .object .as_bytes() - .and_then(|bytes| dual_compiled_contracts.find_evm_bytecode(&bytes.0)) + .and_then(|bytes| dual_compiled_contracts.find_by_evm_bytecode(&bytes.0)) .ok_or(eyre::eyre!( "Could not find zksolc contract for contract {}", self.contract.name @@ -160,7 +153,8 @@ impl CreateArgs { let factory_deps = dual_compiled_contracts .fetch_all_factory_deps(contract) .into_iter() - .collect::>(); + .map(|bc| bc.to_vec()) + .collect(); (abi, zk_bin, Some((contract, factory_deps))) } else { diff --git a/crates/forge/bin/cmd/script/broadcast.rs b/crates/forge/bin/cmd/script/broadcast.rs index 8dce97b61..3bf3a8acf 100644 --- a/crates/forge/bin/cmd/script/broadcast.rs +++ b/crates/forge/bin/cmd/script/broadcast.rs @@ -29,7 +29,7 @@ use foundry_common::{ use foundry_compilers::{artifacts::Libraries, ArtifactId}; use foundry_config::Config; use foundry_wallets::WalletSigner; -use foundry_zksync_compiler::DualCompiledContract; +use foundry_zksync_compiler::{DualCompiledContracts}; use futures::StreamExt; use std::{ cmp::min, @@ -317,7 +317,7 @@ impl ScriptArgs { mut script_config: ScriptConfig, verify: VerifyBundle, signers: &HashMap, - dual_compiled_contracts: Option>, + dual_compiled_contracts: Option, ) -> Result<()> { if let Some(txs) = result.transactions.take() { script_config.collect_rpcs(&txs); @@ -419,7 +419,7 @@ impl ScriptArgs { script_config: &mut ScriptConfig, decoder: &CallTraceDecoder, known_contracts: &ContractsByArtifact, - dual_compiled_contracts: Option>, + dual_compiled_contracts: Option, ) -> Result> { if !txs.is_empty() { let gas_filled_txs = self @@ -458,7 +458,7 @@ impl ScriptArgs { script_config: &ScriptConfig, decoder: &CallTraceDecoder, known_contracts: &ContractsByArtifact, - dual_compiled_contracts: Option>, + dual_compiled_contracts: Option, ) -> Result> { let gas_filled_txs = if self.skip_simulation { shell::println("\nSKIPPING ON CHAIN SIMULATION.")?; diff --git a/crates/forge/bin/cmd/script/build.rs b/crates/forge/bin/cmd/script/build.rs index 038463af2..356b22d77 100644 --- a/crates/forge/bin/cmd/script/build.rs +++ b/crates/forge/bin/cmd/script/build.rs @@ -11,7 +11,7 @@ use foundry_compilers::{ info::ContractInfo, ArtifactId, Project, ProjectCompileOutput, }; -use foundry_zksync_compiler::{new_dual_compiled_contracts, DualCompiledContract, ZkSolc}; +use foundry_zksync_compiler::{DualCompiledContracts, ZkSolc}; use std::str::FromStr; impl ScriptArgs { @@ -46,7 +46,7 @@ impl ScriptArgs { Ok(compiled) => compiled, Err(e) => return Err(eyre::eyre!("Failed to compile with zksolc: {}", e)), }; - let dual_compiled_contracts = new_dual_compiled_contracts(&output, &zk_output); + let dual_compiled_contracts = DualCompiledContracts::new(&output, &zk_output); let sources = ContractSources::from_project_output(&output, root)?; let contracts = output.into_artifacts().collect(); @@ -228,5 +228,5 @@ pub struct BuildOutput { pub libraries: Libraries, pub predeploy_libraries: Vec, pub sources: ContractSources, - pub dual_compiled_contracts: Option>, + pub dual_compiled_contracts: Option, } diff --git a/crates/forge/bin/cmd/script/cmd.rs b/crates/forge/bin/cmd/script/cmd.rs index 5b34638e8..c46dfe1a0 100644 --- a/crates/forge/bin/cmd/script/cmd.rs +++ b/crates/forge/bin/cmd/script/cmd.rs @@ -19,7 +19,7 @@ use foundry_compilers::{ use foundry_debugger::Debugger; use foundry_evm::inspectors::cheatcodes::{BroadcastableTransaction, ScriptWallets}; use foundry_wallets::WalletSigner; -use foundry_zksync_compiler::DualCompiledContract; +use foundry_zksync_compiler::DualCompiledContracts; use std::{collections::HashMap, sync::Arc}; /// Helper alias type for the collection of data changed due to the new sender. @@ -159,7 +159,7 @@ impl ScriptArgs { predeploy_libraries: Vec, result: &mut ScriptResult, script_wallets: ScriptWallets, - dual_compiled_contracts: Option>, + dual_compiled_contracts: Option, ) -> Result> { if let Some(new_sender) = self.maybe_new_sender( &script_config.evm_opts, @@ -332,7 +332,7 @@ impl ScriptArgs { first_run_result: &mut ScriptResult, linker: Linker, script_wallets: ScriptWallets, - dual_compiled_contracts: Option>, + dual_compiled_contracts: Option, ) -> Result<(Libraries, ArtifactContracts)> { // if we had a new sender that requires relinking, we need to // get the nonce mainnet for accurate addresses for predeploy libs diff --git a/crates/forge/bin/cmd/script/executor.rs b/crates/forge/bin/cmd/script/executor.rs index 93652dc5f..5f7eed2eb 100644 --- a/crates/forge/bin/cmd/script/executor.rs +++ b/crates/forge/bin/cmd/script/executor.rs @@ -18,7 +18,7 @@ use foundry_cli::utils::{ensure_clean_constructor, needs_setup}; use foundry_common::{get_contract_name, provider::ethers::RpcUrl, shell, ContractsByArtifact}; use foundry_compilers::artifacts::ContractBytecodeSome; use foundry_evm::inspectors::cheatcodes::ScriptWallets; -use foundry_zksync_compiler::DualCompiledContract; +use foundry_zksync_compiler::{DualCompiledContracts}; use futures::future::join_all; use parking_lot::RwLock; use std::{ @@ -36,7 +36,7 @@ impl ScriptArgs { sender: Address, predeploy_libraries: &[Bytes], script_wallets: ScriptWallets, - dual_compiled_contracts: Option>, + dual_compiled_contracts: Option, ) -> Result { trace!(target: "script", "start executing script"); @@ -104,7 +104,7 @@ impl ScriptArgs { script_config: &ScriptConfig, decoder: &CallTraceDecoder, contracts: &ContractsByArtifact, - dual_compiled_contracts: Option>, + dual_compiled_contracts: Option, ) -> Result> { trace!(target: "script", "executing onchain simulation"); @@ -253,7 +253,7 @@ impl ScriptArgs { async fn build_runners( &self, script_config: &ScriptConfig, - dual_compiled_contracts: Option>, + dual_compiled_contracts: Option, ) -> Result> { let sender = script_config.evm_opts.sender; @@ -292,7 +292,7 @@ impl ScriptArgs { sender: Address, stage: SimulationStage, script_wallets: Option, - dual_compiled_contracts: Option>, + dual_compiled_contracts: Option, ) -> Result { trace!("preparing script runner"); let env = script_config.evm_opts.evm_env().await?; diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index e677c5d98..df6cc0f8a 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -31,7 +31,7 @@ use foundry_config::{ get_available_profiles, Config, }; use foundry_debugger::Debugger; -use foundry_zksync_compiler::{new_dual_compiled_contracts, ZkSolc}; +use foundry_zksync_compiler::{DualCompiledContracts, ZkSolc}; use regex::Regex; use std::{sync::mpsc::channel, time::Instant}; use watchexec::config::{InitConfig, RuntimeConfig}; @@ -185,7 +185,7 @@ impl TestArgs { Ok(compiled) => compiled, Err(e) => return Err(eyre::eyre!("Failed to compile with zksolc: {}", e)), }; - let dual_compiled_contracts = new_dual_compiled_contracts(&output, &zk_output); + let dual_compiled_contracts = DualCompiledContracts::new(&output, &zk_output); // Create test options from general project settings and compiler output. let project_root = &project.paths.root; diff --git a/crates/forge/tests/it/config.rs b/crates/forge/tests/it/config.rs index a18fdfae6..78e5a0c75 100644 --- a/crates/forge/tests/it/config.rs +++ b/crates/forge/tests/it/config.rs @@ -18,7 +18,7 @@ use foundry_evm::{ traces::{render_trace_arena, CallTraceDecoderBuilder}, }; use foundry_test_utils::{init_tracing, Filter}; -use foundry_zksync_compiler::{DualCompiledContract, PackedEraBytecode}; +use foundry_zksync_compiler::{DualCompiledContract, DualCompiledContracts, PackedEraBytecode}; use futures::future::join_all; use itertools::Itertools; use std::{ @@ -208,7 +208,7 @@ pub async fn runner_with_config_and_zk(mut config: Config) -> MultiContractRunne let zk_output = COMPILED_ZK.clone(); // Dual compiled contracts - let mut dual_compiled_contracts = vec![]; + let mut dual_compiled_contracts = DualCompiledContracts::default(); let mut solc_bytecodes = HashMap::new(); for (contract_name, artifact) in output.artifacts() { let contract_name = diff --git a/crates/zksync/compiler/src/zksolc/mod.rs b/crates/zksync/compiler/src/zksolc/mod.rs index b66baecfb..f7601faf0 100644 --- a/crates/zksync/compiler/src/zksolc/mod.rs +++ b/crates/zksync/compiler/src/zksolc/mod.rs @@ -36,93 +36,86 @@ pub struct DualCompiledContract { pub evm_bytecode: Vec, } -/// Creates a list of [DualCompiledContract]s from the provided solc and zksolc output. -pub fn new_dual_compiled_contracts( - output: &ProjectCompileOutput, - zk_output: &ProjectCompileOutput, -) -> Vec { - let mut dual_compiled_contracts = vec![]; - let mut solc_bytecodes = HashMap::new(); - for (contract_name, artifact) in output.artifacts() { - let contract_name = - contract_name.split('.').next().expect("name cannot be empty").to_string(); - let deployed_bytecode = artifact.get_deployed_bytecode(); - let deployed_bytecode = deployed_bytecode - .as_ref() - .and_then(|d| d.bytecode.as_ref().and_then(|b| b.object.as_bytes())); - let bytecode = artifact.get_bytecode().and_then(|b| b.object.as_bytes().cloned()); - if let Some(bytecode) = bytecode { - if let Some(deployed_bytecode) = deployed_bytecode { - solc_bytecodes.insert(contract_name.clone(), (bytecode, deployed_bytecode.clone())); +/// A collection of `[DualCompiledContract]`s +#[derive(Debug, Default, Clone)] +pub struct DualCompiledContracts { + contracts: Vec, +} + +impl DualCompiledContracts { + /// Creates a collection of `[DualCompiledContract]`s from the provided solc and zksolc output. + pub fn new(output: &ProjectCompileOutput, zk_output: &ProjectCompileOutput) -> Self { + let mut dual_compiled_contracts = vec![]; + let mut solc_bytecodes = HashMap::new(); + for (contract_name, artifact) in output.artifacts() { + let contract_name = + contract_name.split('.').next().expect("name cannot be empty").to_string(); + let deployed_bytecode = artifact.get_deployed_bytecode(); + let deployed_bytecode = deployed_bytecode + .as_ref() + .and_then(|d| d.bytecode.as_ref().and_then(|b| b.object.as_bytes())); + let bytecode = artifact.get_bytecode().and_then(|b| b.object.as_bytes().cloned()); + if let Some(bytecode) = bytecode { + if let Some(deployed_bytecode) = deployed_bytecode { + solc_bytecodes + .insert(contract_name.clone(), (bytecode, deployed_bytecode.clone())); + } } } - } - for (contract_name, artifact) in zk_output.artifacts() { - let deployed_bytecode = artifact.get_deployed_bytecode(); - let deployed_bytecode = deployed_bytecode - .as_ref() - .and_then(|d| d.bytecode.as_ref().and_then(|b| b.object.as_bytes())); - if let Some(deployed_bytecode) = deployed_bytecode { - let packed_bytecode = PackedEraBytecode::from_vec(deployed_bytecode); - if let Some((solc_bytecode, solc_deployed_bytecode)) = - solc_bytecodes.get(&contract_name) - { - dual_compiled_contracts.push(DualCompiledContract { - name: contract_name, - zk_bytecode_hash: packed_bytecode.bytecode_hash(), - zk_deployed_bytecode: packed_bytecode.bytecode(), - zk_factory_deps: packed_bytecode.factory_deps(), - evm_bytecode_hash: keccak256(solc_deployed_bytecode), - evm_bytecode: solc_bytecode.to_vec(), - evm_deployed_bytecode: solc_deployed_bytecode.to_vec(), - }); + for (contract_name, artifact) in zk_output.artifacts() { + let deployed_bytecode = artifact.get_deployed_bytecode(); + let deployed_bytecode = deployed_bytecode + .as_ref() + .and_then(|d| d.bytecode.as_ref().and_then(|b| b.object.as_bytes())); + if let Some(deployed_bytecode) = deployed_bytecode { + let packed_bytecode = PackedEraBytecode::from_vec(deployed_bytecode); + if let Some((solc_bytecode, solc_deployed_bytecode)) = + solc_bytecodes.get(&contract_name) + { + dual_compiled_contracts.push(DualCompiledContract { + name: contract_name, + zk_bytecode_hash: packed_bytecode.bytecode_hash(), + zk_deployed_bytecode: packed_bytecode.bytecode(), + zk_factory_deps: packed_bytecode.factory_deps(), + evm_bytecode_hash: keccak256(solc_deployed_bytecode), + evm_bytecode: solc_bytecode.to_vec(), + evm_deployed_bytecode: solc_deployed_bytecode.to_vec(), + }); + } } } - } - - dual_compiled_contracts -} - -/// Implements methods to look for contracts -pub trait FindContract { - /// Finds a contract matching the EVM bytecode - fn find_evm_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract>; - /// Finds a contract matching the ZK bytecode hash - fn find_zk_bytecode_hash(&self, code_hash: H256) -> Option<&DualCompiledContract>; + Self { contracts: dual_compiled_contracts } + } /// Finds a contract matching the ZK deployed bytecode - fn find_zk_deployed_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract>; - - /// Finds a contract own and nested factory deps - fn fetch_all_factory_deps(&self, root: &DualCompiledContract) -> HashSet>; -} - -impl FindContract for Vec { - fn find_zk_deployed_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract> { - self.iter().find(|contract| bytecode.starts_with(&contract.zk_deployed_bytecode)) + pub fn find_by_zk_deployed_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract> { + self.contracts.iter().find(|contract| bytecode.starts_with(&contract.zk_deployed_bytecode)) } - fn find_evm_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract> { - self.iter().find(|contract| bytecode.starts_with(&contract.evm_bytecode)) + /// Finds a contract matching the EVM bytecode + pub fn find_by_evm_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract> { + self.contracts.iter().find(|contract| bytecode.starts_with(&contract.evm_bytecode)) } - fn find_zk_bytecode_hash(&self, code_hash: H256) -> Option<&DualCompiledContract> { - self.iter().find(|contract| code_hash == contract.zk_bytecode_hash) + /// Finds a contract matching the ZK bytecode hash + pub fn find_by_zk_bytecode_hash(&self, code_hash: H256) -> Option<&DualCompiledContract> { + self.contracts.iter().find(|contract| code_hash == contract.zk_bytecode_hash) } - fn fetch_all_factory_deps(&self, root: &DualCompiledContract) -> HashSet> { + /// Finds a contract own and nested factory deps + pub fn fetch_all_factory_deps<'s>(&'s self, root: &'s DualCompiledContract) -> HashSet<&[u8]> { let mut visited = HashSet::new(); let mut queue = VecDeque::new(); - for dep in root.zk_factory_deps.iter().cloned() { - queue.push_back(dep); + for dep in &root.zk_factory_deps { + queue.push_back(dep.as_slice()); } while let Some(dep) = queue.pop_front() { //try to insert in the list of visited, if it's already present, skip - if visited.insert(dep.clone()) { - if let Some(contract) = self.find_zk_deployed_bytecode(&dep) { + if visited.insert(dep) { + if let Some(contract) = self.find_by_zk_deployed_bytecode(dep) { debug!( name = contract.name, deps = contract.zk_factory_deps.len(), @@ -131,9 +124,9 @@ impl FindContract for Vec { for nested_dep in &contract.zk_factory_deps { //check that the nested dependency is inserted - if !visited.contains(nested_dep) { + if !visited.contains(nested_dep.as_slice()) { //if not, add it to queue for processing - queue.push_back(nested_dep.clone()); + queue.push_back(nested_dep.as_slice()); } } } @@ -142,50 +135,14 @@ impl FindContract for Vec { visited } -} - -impl FindContract for &[DualCompiledContract] { - fn find_zk_deployed_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract> { - self.iter().find(|contract| bytecode.starts_with(&contract.zk_deployed_bytecode)) - } - fn find_evm_bytecode(&self, bytecode: &[u8]) -> Option<&DualCompiledContract> { - self.iter().find(|contract| bytecode.starts_with(&contract.evm_bytecode)) + /// Returns an iterator over all `[DualCompiledContract]`s in the collection + pub fn iter(&self) -> impl Iterator { + self.contracts.iter() } - fn find_zk_bytecode_hash(&self, code_hash: H256) -> Option<&DualCompiledContract> { - self.iter().find(|contract| code_hash == contract.zk_bytecode_hash) - } - - fn fetch_all_factory_deps(&self, root: &DualCompiledContract) -> HashSet> { - let mut visited = HashSet::new(); - let mut queue = VecDeque::new(); - - for dep in root.zk_factory_deps.iter().cloned() { - queue.push_back(dep); - } - - while let Some(dep) = queue.pop_front() { - //try to insert in the list of visited, if it's already present, skip - if visited.insert(dep.clone()) { - if let Some(contract) = self.find_zk_deployed_bytecode(&dep) { - debug!( - name = contract.name, - deps = contract.zk_factory_deps.len(), - "new factory depdendency" - ); - - for nested_dep in &contract.zk_factory_deps { - //check that the nested dependency is inserted - if !visited.contains(nested_dep) { - //if not, add it to queue for processing - queue.push_back(nested_dep.clone()); - } - } - } - } - } - - visited + /// Adds a new `[DualCompiledContract]` to the collection + pub fn push(&mut self, contract: DualCompiledContract) { + self.contracts.push(contract); } } diff --git a/crates/zksync/core/src/vm/runner.rs b/crates/zksync/core/src/vm/runner.rs index 9e4d46ef5..833507c3a 100644 --- a/crates/zksync/core/src/vm/runner.rs +++ b/crates/zksync/core/src/vm/runner.rs @@ -16,7 +16,7 @@ use foundry_common::{ console::HARDHAT_CONSOLE_ADDRESS, fmt::ConsoleFmt, patch_hh_console_selector, Console, HardhatConsole, }; -use foundry_zksync_compiler::{DualCompiledContract, FindContract}; +use foundry_zksync_compiler::DualCompiledContract; use std::{collections::HashMap, fmt::Debug, str::FromStr, sync::Arc}; use crate::{fix_l2_gas_limit, fix_l2_gas_price}; @@ -164,7 +164,7 @@ where pub fn create<'a, DB, E>( call: &CreateInputs, contract: &DualCompiledContract, - dual_compiled_contracts: &[DualCompiledContract], + factory_deps: Vec>, env: &'a mut Env, db: &'a mut DB, journaled_state: &'a mut JournaledState, @@ -178,8 +178,6 @@ where let constructor_input = call.init_code[contract.evm_bytecode.len()..].to_vec(); let caller = env.tx.caller; let calldata = encode_create_params(&call.scheme, contract.zk_bytecode_hash, constructor_input); - let factory_deps = - dual_compiled_contracts.fetch_all_factory_deps(contract).into_iter().collect(); let nonce = ZKVMData::new(db, journaled_state).get_tx_nonce(caller); let (gas_limit, max_fee_per_gas) = gas_params(env, db, journaled_state, caller); From 78199a4fea07c1dfe2dc64044b0d5a851186348f Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Fri, 12 Apr 2024 17:44:10 +0200 Subject: [PATCH 10/23] fix(test:zk): Factory contract cleanup --- zk-tests/src/Factory.t.sol | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/zk-tests/src/Factory.t.sol b/zk-tests/src/Factory.t.sol index 93d1854ef..7a1cadbc3 100644 --- a/zk-tests/src/Factory.t.sol +++ b/zk-tests/src/Factory.t.sol @@ -42,7 +42,7 @@ contract MyConstructorFactory { contract MyNestedFactory { MyClassicFactory nested; - function create(uint256 _number) { + function create(uint256 _number) public { nested = new MyClassicFactory(); nested.create(_number); @@ -72,7 +72,7 @@ contract MyUserFactory { MyClassicFactory(classicFactory).create(_number); } - function getNumber(address classicFactory) public returns (uint256) { + function getNumber(address classicFactory) public view returns (uint256) { return MyClassicFactory(classicFactory).getNumber(); } } @@ -82,7 +82,7 @@ contract MyUserConstructorFactory { MyClassicFactory(classicFactory).create(_number); } - function getNumber(address classicFactory) public returns (uint256) { + function getNumber(address classicFactory) public view returns (uint256) { return MyClassicFactory(classicFactory).getNumber(); } } @@ -116,16 +116,16 @@ contract ZkFactory is Test { function testUserFactory() public { MyClassicFactory factory = new MyClassicFactory(); - MyUserFactory user = new MyUserFactory(address(factory)); - user.create(42); + MyUserFactory user = new MyUserFactory(); + user.create(address(factory), 42); - assert(user.getNumber() == 42); + assert(user.getNumber(address(factory)) == 42); } function testUserConstructorFactory() public { MyClassicFactory factory = new MyClassicFactory(); - MyUserConstructorFactory factory = new MyUserConstructorFactory(address(factory), 42); + MyUserConstructorFactory user = new MyUserConstructorFactory(address(factory), 42); - assert(factory.getNumber() == 42); + assert(user.getNumber(address(factory)) == 42); } } From 5a1a8b36ee3f8b2bb83aaf42e63cace7b2bcceb8 Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Fri, 12 Apr 2024 18:27:05 +0200 Subject: [PATCH 11/23] fix(call:zk): pass factory deps fix(test:zk): proper user factory --- crates/cheatcodes/src/inspector.rs | 12 +++++++++++- crates/zksync/core/src/vm/runner.rs | 3 +-- zk-tests/src/Factory.t.sol | 14 ++------------ 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/crates/cheatcodes/src/inspector.rs b/crates/cheatcodes/src/inspector.rs index 389ff241e..dc62cd0e6 100644 --- a/crates/cheatcodes/src/inspector.rs +++ b/crates/cheatcodes/src/inspector.rs @@ -1193,25 +1193,35 @@ impl Inspector for Cheatcodes { .load_account(call.contract, data.db) .map(|(account, _)| account.info.code_hash) .unwrap_or_default(); + let contract = if code_hash != KECCAK_EMPTY { self.dual_compiled_contracts .find_by_zk_bytecode_hash(zksync_types::H256::from(code_hash.0)) } else { None }; + if let Some(contract) = contract { tracing::debug!(contract = contract.name, "using dual compiled contract"); } else { error!("no zk contract was found for {code_hash:?}"); } + let factory_deps = contract.map(|contract| { + self.dual_compiled_contracts + .fetch_all_factory_deps(contract) + .into_iter() + .map(|bc| bc.to_vec()) + .collect() + }); + let ccx = foundry_zksync_core::vm::CheatcodeTracerContext { mocked_calls: self.mocked_calls.clone(), expected_calls: Some(&mut self.expected_calls), }; if let Ok(result) = foundry_zksync_core::vm::call::<_, DatabaseError>( call, - contract, + factory_deps, data.env, data.db, &mut data.journaled_state, diff --git a/crates/zksync/core/src/vm/runner.rs b/crates/zksync/core/src/vm/runner.rs index 833507c3a..402b792e4 100644 --- a/crates/zksync/core/src/vm/runner.rs +++ b/crates/zksync/core/src/vm/runner.rs @@ -202,7 +202,7 @@ where /// Executes a CALL opcode on the ZK-VM. pub fn call<'a, DB, E>( call: &CallInputs, - contract: Option<&DualCompiledContract>, + factory_deps: Option>>, env: &'a mut Env, db: &'a mut DB, journaled_state: &'a mut JournaledState, @@ -215,7 +215,6 @@ where info!(?call, "call tx {}", hex::encode(&call.input)); let msg_sender = call.context.caller; let caller = env.tx.caller; - let factory_deps = contract.map(|contract| vec![contract.zk_deployed_bytecode.clone()]); let nonce: zksync_types::Nonce = ZKVMData::new(db, journaled_state).get_tx_nonce(caller); let (gas_limit, max_fee_per_gas) = gas_params(env, db, journaled_state, caller); diff --git a/zk-tests/src/Factory.t.sol b/zk-tests/src/Factory.t.sol index 7a1cadbc3..efc5a6d72 100644 --- a/zk-tests/src/Factory.t.sol +++ b/zk-tests/src/Factory.t.sol @@ -77,16 +77,6 @@ contract MyUserFactory { } } -contract MyUserConstructorFactory { - constructor(address classicFactory, uint256 _number) { - MyClassicFactory(classicFactory).create(_number); - } - - function getNumber(address classicFactory) public view returns (uint256) { - return MyClassicFactory(classicFactory).getNumber(); - } -} - contract ZkFactory is Test { function testClassicFactory() public { MyClassicFactory factory = new MyClassicFactory(); @@ -123,8 +113,8 @@ contract ZkFactory is Test { } function testUserConstructorFactory() public { - MyClassicFactory factory = new MyClassicFactory(); - MyUserConstructorFactory user = new MyUserConstructorFactory(address(factory), 42); + MyConstructorFactory factory = new MyConstructorFactory(42); + MyUserFactory user = new MyUserFactory(); assert(user.getNumber(address(factory)) == 42); } From b77617130410c573c264c3b48607459e427cb141 Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Fri, 12 Apr 2024 20:13:53 +0200 Subject: [PATCH 12/23] chore: fmt --- crates/forge/bin/cmd/script/broadcast.rs | 2 +- crates/forge/bin/cmd/script/executor.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/forge/bin/cmd/script/broadcast.rs b/crates/forge/bin/cmd/script/broadcast.rs index 3bf3a8acf..9480e000d 100644 --- a/crates/forge/bin/cmd/script/broadcast.rs +++ b/crates/forge/bin/cmd/script/broadcast.rs @@ -29,7 +29,7 @@ use foundry_common::{ use foundry_compilers::{artifacts::Libraries, ArtifactId}; use foundry_config::Config; use foundry_wallets::WalletSigner; -use foundry_zksync_compiler::{DualCompiledContracts}; +use foundry_zksync_compiler::DualCompiledContracts; use futures::StreamExt; use std::{ cmp::min, diff --git a/crates/forge/bin/cmd/script/executor.rs b/crates/forge/bin/cmd/script/executor.rs index 5f7eed2eb..108cdf286 100644 --- a/crates/forge/bin/cmd/script/executor.rs +++ b/crates/forge/bin/cmd/script/executor.rs @@ -18,7 +18,7 @@ use foundry_cli::utils::{ensure_clean_constructor, needs_setup}; use foundry_common::{get_contract_name, provider::ethers::RpcUrl, shell, ContractsByArtifact}; use foundry_compilers::artifacts::ContractBytecodeSome; use foundry_evm::inspectors::cheatcodes::ScriptWallets; -use foundry_zksync_compiler::{DualCompiledContracts}; +use foundry_zksync_compiler::DualCompiledContracts; use futures::future::join_all; use parking_lot::RwLock; use std::{ From b274948031e1ee3355ab505ab687a07c995f3750 Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Fri, 12 Apr 2024 20:21:54 +0200 Subject: [PATCH 13/23] fix(zk:create): restore handle `--factory-deps` feat(zk:create): lookup all factory deps of manually specified deps --- crates/forge/bin/cmd/create.rs | 44 +++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/crates/forge/bin/cmd/create.rs b/crates/forge/bin/cmd/create.rs index 044907305..6ba41c3d3 100644 --- a/crates/forge/bin/cmd/create.rs +++ b/crates/forge/bin/cmd/create.rs @@ -148,15 +148,41 @@ impl CreateArgs { source_map: Default::default(), }; - //TODO: restore `--factory-deps` handling? (+ lookup) - // or remove flag entirely? - let factory_deps = dual_compiled_contracts - .fetch_all_factory_deps(contract) - .into_iter() - .map(|bc| bc.to_vec()) - .collect(); - - (abi, zk_bin, Some((contract, factory_deps))) + let mut factory_deps = dual_compiled_contracts.fetch_all_factory_deps(contract); + + //for manual specified factory deps + for mut contract in std::mem::take(&mut self.factory_deps) { + if let Some(path) = contract.path.as_mut() { + *path = canonicalized(project.root().join(&path)).to_string_lossy().to_string(); + } + + let (_, bin, _) = remove_contract(&mut output, &contract).with_context(|| { + format!("Unable to find specified factory deps ({}) in project", contract.name) + })?; + + let zk = bin + .object + .as_bytes() + .and_then(|bytes| dual_compiled_contracts.find_by_evm_bytecode(&bytes.0)) + .ok_or(eyre::eyre!( + "Could not find zksolc contract for contract {}", + contract.name + ))?; + + //if the dep isn't already present, + // fetch all deps and add them to the final list + if !factory_deps.contains(zk.zk_deployed_bytecode.as_slice()) { + let additional_factory_deps = + dual_compiled_contracts.fetch_all_factory_deps(zk); + factory_deps.extend(additional_factory_deps); + } + } + + ( + abi, + zk_bin, + Some((contract, factory_deps.into_iter().map(|bc| bc.to_vec()).collect())), + ) } else { (abi, bin, None) }; From fdcf193d9a6434a43ddc7efd7104cef26768d917 Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Mon, 15 Apr 2024 16:04:02 +0200 Subject: [PATCH 14/23] refactory(test:zk): split Factory tests & sources feat(test:zk): add scripts for factory tests fix(test:zk): comment out non working test & add FIXME --- zk-tests/script/Factory.s.sol | 72 ++++++++++++++++++++++++++++ zk-tests/src/Factory.sol | 76 ++++++++++++++++++++++++++++++ zk-tests/src/Factory.t.sol | 89 ++++------------------------------- 3 files changed, 157 insertions(+), 80 deletions(-) create mode 100644 zk-tests/script/Factory.s.sol create mode 100644 zk-tests/src/Factory.sol diff --git a/zk-tests/script/Factory.s.sol b/zk-tests/script/Factory.s.sol new file mode 100644 index 000000000..b0ef29adb --- /dev/null +++ b/zk-tests/script/Factory.s.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity ^0.8.0; + +import {Script} from 'forge-std/Script.sol'; + +import '../src/Factory.sol'; + +contract ZkClassicFactoryScript is Script { + function run() external { + vm.startBroadcast(); + MyClassicFactory factory = new MyClassicFactory(); + factory.create(42); + + vm.stopBroadcast(); + assert(factory.getNumber() == 42); + } +} + +contract ZkConstructorFactoryScript is Script { + function run() external { + vm.startBroadcast(); + MyConstructorFactory factory = new MyConstructorFactory(42); + + vm.stopBroadcast(); + assert(factory.getNumber() == 42); + } +} + +contract ZkNestedFactoryScript is Script{ + function run() external { + vm.startBroadcast(); + MyNestedFactory factory = new MyNestedFactory(); + factory.create(42); + + vm.stopBroadcast(); + assert(factory.getNumber() == 42); + } +} + +contract ZkNestedConstructorFactoryScript is Script{ + function run() external { + vm.startBroadcast(); + MyNestedConstructorFactory factory = new MyNestedConstructorFactory(42); + + vm.stopBroadcast(); + assert(factory.getNumber() == 42); + } +} + +//FIXME: fails with 'trying to decode unexisting hash' +contract ZkUserFactoryScript is Script { + function run() external { + vm.startBroadcast(); + MyClassicFactory factory = new MyClassicFactory(); + MyUserFactory user = new MyUserFactory(); + user.create(address(factory), 42); + + vm.stopBroadcast(); + assert(user.getNumber(address(factory)) == 42); + } +} + +contract ZkUserConstructorFactoryScript is Script{ + function run() external { + vm.startBroadcast(); + MyConstructorFactory factory = new MyConstructorFactory(42); + MyUserFactory user = new MyUserFactory(); + + vm.stopBroadcast(); + assert(user.getNumber(address(factory)) == 42); + } +} diff --git a/zk-tests/src/Factory.sol b/zk-tests/src/Factory.sol new file mode 100644 index 000000000..68e93dc74 --- /dev/null +++ b/zk-tests/src/Factory.sol @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity ^0.8.0; + +/// Set of tests for factory contracts +/// +/// *Constructor factories build their dependencies in their constructors +/// *User factories don't deploy but assume the given address to be a deployed factory + +contract MyContract { + uint256 public number; + constructor(uint256 _number) { + number = _number; + } +} + +contract MyClassicFactory { + MyContract item; + + function create(uint256 _number) public { + item = new MyContract(_number); + } + + function getNumber() public view returns (uint256) { + return item.number(); + } +} + +contract MyConstructorFactory { + MyContract item; + + constructor(uint256 _number) { + item = new MyContract(_number); + } + + function getNumber() public view returns (uint256) { + return item.number(); + } +} + +contract MyNestedFactory { + MyClassicFactory nested; + + function create(uint256 _number) public { + nested = new MyClassicFactory(); + + nested.create(_number); + } + + function getNumber() public view returns (uint256) { + return nested.getNumber(); + } +} + +contract MyNestedConstructorFactory { + MyClassicFactory nested; + + constructor(uint256 _number) { + nested = new MyClassicFactory(); + + nested.create(_number); + } + + function getNumber() public view returns (uint256) { + return nested.getNumber(); + } +} + +contract MyUserFactory { + function create(address classicFactory, uint256 _number) public { + MyClassicFactory(classicFactory).create(_number); + } + + function getNumber(address classicFactory) public view returns (uint256) { + return MyClassicFactory(classicFactory).getNumber(); + } +} diff --git a/zk-tests/src/Factory.t.sol b/zk-tests/src/Factory.t.sol index efc5a6d72..6f8a9551a 100644 --- a/zk-tests/src/Factory.t.sol +++ b/zk-tests/src/Factory.t.sol @@ -3,81 +3,9 @@ pragma solidity ^0.8.0; import {Test} from 'forge-std/Test.sol'; -/// Set of tests for factory contracts -/// -/// *Constructor factories build their dependencies in their constructors -/// *User factories don't deploy but assume the given address to be a deployed factory +import './Factory.sol'; -contract MyContract { - uint256 public number; - constructor(uint256 _number) { - number = _number; - } -} - -contract MyClassicFactory { - MyContract item; - - function create(uint256 _number) public { - item = new MyContract(_number); - } - - function getNumber() public view returns (uint256) { - return item.number(); - } -} - -contract MyConstructorFactory { - MyContract item; - - constructor(uint256 _number) { - item = new MyContract(_number); - } - - function getNumber() public view returns (uint256) { - return item.number(); - } -} - -contract MyNestedFactory { - MyClassicFactory nested; - - function create(uint256 _number) public { - nested = new MyClassicFactory(); - - nested.create(_number); - } - - function getNumber() public view returns (uint256) { - return nested.getNumber(); - } -} - -contract MyNestedConstructorFactory { - MyClassicFactory nested; - - constructor(uint256 _number) { - nested = new MyClassicFactory(); - - nested.create(_number); - } - - function getNumber() public view returns (uint256) { - return nested.getNumber(); - } -} - -contract MyUserFactory { - function create(address classicFactory, uint256 _number) public { - MyClassicFactory(classicFactory).create(_number); - } - - function getNumber(address classicFactory) public view returns (uint256) { - return MyClassicFactory(classicFactory).getNumber(); - } -} - -contract ZkFactory is Test { +contract ZkFactoryTest is Test { function testClassicFactory() public { MyClassicFactory factory = new MyClassicFactory(); factory.create(42); @@ -104,13 +32,14 @@ contract ZkFactory is Test { assert(factory.getNumber() == 42); } - function testUserFactory() public { - MyClassicFactory factory = new MyClassicFactory(); - MyUserFactory user = new MyUserFactory(); - user.create(address(factory), 42); + // //FIXME: fails with 'trying to decode unexisting hash' + // function testUserFactory() public { + // MyClassicFactory factory = new MyClassicFactory(); + // MyUserFactory user = new MyUserFactory(); + // user.create(address(factory), 42); - assert(user.getNumber(address(factory)) == 42); - } + // assert(user.getNumber(address(factory)) == 42); + // } function testUserConstructorFactory() public { MyConstructorFactory factory = new MyConstructorFactory(42); From e7cbe94a3a6563dd483ea59978f8d219e54a8633 Mon Sep 17 00:00:00 2001 From: Karrq Date: Tue, 16 Apr 2024 14:49:19 +0200 Subject: [PATCH 15/23] chore(docs): formatting Co-authored-by: Nisheeth Barthwal --- crates/forge/bin/cmd/create.rs | 2 +- crates/zksync/compiler/src/zksolc/mod.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/forge/bin/cmd/create.rs b/crates/forge/bin/cmd/create.rs index 6ba41c3d3..3841bade1 100644 --- a/crates/forge/bin/cmd/create.rs +++ b/crates/forge/bin/cmd/create.rs @@ -169,7 +169,7 @@ impl CreateArgs { contract.name ))?; - //if the dep isn't already present, + // if the dep isn't already present, // fetch all deps and add them to the final list if !factory_deps.contains(zk.zk_deployed_bytecode.as_slice()) { let additional_factory_deps = diff --git a/crates/zksync/compiler/src/zksolc/mod.rs b/crates/zksync/compiler/src/zksolc/mod.rs index f7601faf0..58a89becc 100644 --- a/crates/zksync/compiler/src/zksolc/mod.rs +++ b/crates/zksync/compiler/src/zksolc/mod.rs @@ -113,7 +113,7 @@ impl DualCompiledContracts { } while let Some(dep) = queue.pop_front() { - //try to insert in the list of visited, if it's already present, skip + // try to insert in the list of visited, if it's already present, skip if visited.insert(dep) { if let Some(contract) = self.find_by_zk_deployed_bytecode(dep) { debug!( @@ -123,9 +123,9 @@ impl DualCompiledContracts { ); for nested_dep in &contract.zk_factory_deps { - //check that the nested dependency is inserted + // check that the nested dependency is inserted if !visited.contains(nested_dep.as_slice()) { - //if not, add it to queue for processing + // if not, add it to queue for processing queue.push_back(nested_dep.as_slice()); } } From 57a8720638a705f5cbebd6c5db547d0da7cc8064 Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Tue, 16 Apr 2024 17:47:02 +0200 Subject: [PATCH 16/23] feat(zkvm): persist seen factory deps fix(tests:zk): enable `testUserFactory` refactor(compile:zk): `fetch_all_factory_deps` return `Vec>` --- crates/cheatcodes/src/fs.rs | 6 ++++- crates/cheatcodes/src/inspector.rs | 29 ++++++++++------------- crates/cheatcodes/src/utils.rs | 6 ++++- crates/forge/bin/cmd/create.rs | 3 ++- crates/zksync/compiler/src/zksolc/mod.rs | 10 ++++---- crates/zksync/core/src/lib.rs | 1 + crates/zksync/core/src/vm/runner.rs | 23 +++++++++--------- crates/zksync/core/src/vm/storage_view.rs | 25 +++++++++++++++---- crates/zksync/core/src/vm/tracer.rs | 2 ++ zk-tests/src/Factory.t.sol | 15 ++++++------ 10 files changed, 71 insertions(+), 49 deletions(-) diff --git a/crates/cheatcodes/src/fs.rs b/crates/cheatcodes/src/fs.rs index 3e345db94..fa635af60 100644 --- a/crates/cheatcodes/src/fs.rs +++ b/crates/cheatcodes/src/fs.rs @@ -382,7 +382,11 @@ mod tests { root: PathBuf::from(&env!("CARGO_MANIFEST_DIR")), ..Default::default() }; - Cheatcodes { config: Arc::new(config), ..Default::default() } + + let mut cheatcodes = Cheatcodes::default(); + cheatcodes.config = Arc::new(config); + + cheatcodes } #[test] diff --git a/crates/cheatcodes/src/inspector.rs b/crates/cheatcodes/src/inspector.rs index dc62cd0e6..89dc538bf 100644 --- a/crates/cheatcodes/src/inspector.rs +++ b/crates/cheatcodes/src/inspector.rs @@ -31,7 +31,7 @@ use foundry_evm_core::{ use foundry_zksync_compiler::DualCompiledContracts; use foundry_zksync_core::{ convert::{ConvertAddress, ConvertH160, ConvertH256, ConvertRU256, ConvertU256}, - ZkTransactionMetadata, + hash_bytecode, ZkTransactionMetadata, }; use itertools::Itertools; use revm::{ @@ -58,7 +58,7 @@ use zksync_types::{ get_code_key, get_nonce_key, utils::{decompose_full_nonce, nonces_to_full_nonce, storage_key_for_eth_balance}, ACCOUNT_CODE_STORAGE_ADDRESS, CONTRACT_DEPLOYER_ADDRESS, CURRENT_VIRTUAL_BLOCK_INFO_POSITION, - KNOWN_CODES_STORAGE_ADDRESS, L2_ETH_TOKEN_ADDRESS, NONCE_HOLDER_ADDRESS, + H256, KNOWN_CODES_STORAGE_ADDRESS, L2_ETH_TOKEN_ADDRESS, NONCE_HOLDER_ADDRESS, SYSTEM_CONTEXT_ADDRESS, }; @@ -232,6 +232,7 @@ pub struct Cheatcodes { /// Dual compiled contracts pub dual_compiled_contracts: DualCompiledContracts, + persisted_factory_deps: HashMap>, /// Logs printed during ZK-VM execution. /// EVM logs have the value `None` so they can be interpolated later, since @@ -1207,21 +1208,15 @@ impl Inspector for Cheatcodes { error!("no zk contract was found for {code_hash:?}"); } - let factory_deps = contract.map(|contract| { - self.dual_compiled_contracts - .fetch_all_factory_deps(contract) - .into_iter() - .map(|bc| bc.to_vec()) - .collect() - }); + let persisted_factory_deps = self.persisted_factory_deps.clone(); let ccx = foundry_zksync_core::vm::CheatcodeTracerContext { mocked_calls: self.mocked_calls.clone(), expected_calls: Some(&mut self.expected_calls), + persisted_factory_deps, }; if let Ok(result) = foundry_zksync_core::vm::call::<_, DatabaseError>( call, - factory_deps, data.env, data.db, &mut data.journaled_state, @@ -1691,17 +1686,19 @@ impl Inspector for Cheatcodes { .find_by_evm_bytecode(&call.init_code.0) .unwrap_or_else(|| panic!("failed finding contract for {:?}", call.init_code)); - let factory_deps = self - .dual_compiled_contracts - .fetch_all_factory_deps(zk_contract) - .into_iter() - .map(|bc| bc.to_vec()) - .collect(); + let factory_deps = self.dual_compiled_contracts.fetch_all_factory_deps(zk_contract); + + // get the current persisted factory deps to pass to zk create + let persisted_factory_deps = self.persisted_factory_deps.clone(); + // and extend it for future calls + self.persisted_factory_deps + .extend(factory_deps.iter().map(|dep| (hash_bytecode(dep), dep.clone()))); tracing::debug!(contract = zk_contract.name, "using dual compiled contract"); let ccx = foundry_zksync_core::vm::CheatcodeTracerContext { mocked_calls: self.mocked_calls.clone(), expected_calls: Some(&mut self.expected_calls), + persisted_factory_deps, }; if let Ok(result) = foundry_zksync_core::vm::create::<_, DatabaseError>( call, diff --git a/crates/cheatcodes/src/utils.rs b/crates/cheatcodes/src/utils.rs index 525120d7d..08ff7dc21 100644 --- a/crates/cheatcodes/src/utils.rs +++ b/crates/cheatcodes/src/utils.rs @@ -252,7 +252,11 @@ mod tests { root: PathBuf::from(&env!("CARGO_MANIFEST_DIR")), ..Default::default() }; - Cheatcodes { config: Arc::new(config), ..Default::default() } + + let mut cheatcodes = Cheatcodes::default(); + cheatcodes.config = Arc::new(config); + + cheatcodes } #[test] diff --git a/crates/forge/bin/cmd/create.rs b/crates/forge/bin/cmd/create.rs index 3841bade1..3aa160ce6 100644 --- a/crates/forge/bin/cmd/create.rs +++ b/crates/forge/bin/cmd/create.rs @@ -171,10 +171,11 @@ impl CreateArgs { // if the dep isn't already present, // fetch all deps and add them to the final list - if !factory_deps.contains(zk.zk_deployed_bytecode.as_slice()) { + if !factory_deps.contains(&zk.zk_deployed_bytecode) { let additional_factory_deps = dual_compiled_contracts.fetch_all_factory_deps(zk); factory_deps.extend(additional_factory_deps); + factory_deps.dedup(); } } diff --git a/crates/zksync/compiler/src/zksolc/mod.rs b/crates/zksync/compiler/src/zksolc/mod.rs index 58a89becc..b35c86a69 100644 --- a/crates/zksync/compiler/src/zksolc/mod.rs +++ b/crates/zksync/compiler/src/zksolc/mod.rs @@ -104,12 +104,12 @@ impl DualCompiledContracts { } /// Finds a contract own and nested factory deps - pub fn fetch_all_factory_deps<'s>(&'s self, root: &'s DualCompiledContract) -> HashSet<&[u8]> { + pub fn fetch_all_factory_deps(&self, root: &DualCompiledContract) -> Vec> { let mut visited = HashSet::new(); let mut queue = VecDeque::new(); for dep in &root.zk_factory_deps { - queue.push_back(dep.as_slice()); + queue.push_back(dep); } while let Some(dep) = queue.pop_front() { @@ -124,16 +124,16 @@ impl DualCompiledContracts { for nested_dep in &contract.zk_factory_deps { // check that the nested dependency is inserted - if !visited.contains(nested_dep.as_slice()) { + if !visited.contains(nested_dep) { // if not, add it to queue for processing - queue.push_back(nested_dep.as_slice()); + queue.push_back(nested_dep); } } } } } - visited + visited.into_iter().cloned().collect() } /// Returns an iterator over all `[DualCompiledContract]`s in the collection diff --git a/crates/zksync/core/src/lib.rs b/crates/zksync/core/src/lib.rs index 027105738..19a8e1de6 100644 --- a/crates/zksync/core/src/lib.rs +++ b/crates/zksync/core/src/lib.rs @@ -29,6 +29,7 @@ pub use zksync_types::{ ACCOUNT_CODE_STORAGE_ADDRESS, CONTRACT_DEPLOYER_ADDRESS, L2_ETH_TOKEN_ADDRESS, NONCE_HOLDER_ADDRESS, }; +pub use zksync_utils::bytecode::hash_bytecode; use zksync_web3_rs::{ eip712::{Eip712Meta, Eip712Transaction, Eip712TransactionRequest}, providers::Middleware, diff --git a/crates/zksync/core/src/vm/runner.rs b/crates/zksync/core/src/vm/runner.rs index 402b792e4..d8ad055f7 100644 --- a/crates/zksync/core/src/vm/runner.rs +++ b/crates/zksync/core/src/vm/runner.rs @@ -41,15 +41,11 @@ use tracing::{info, trace}; use zksync_basic_types::{L2ChainId, H256}; use zksync_state::{ReadStorage, StoragePtr, WriteStorage}; use zksync_types::{ - ethabi::{self}, - fee::Fee, - l2::L2Tx, - transaction_request::PaymasterParams, - vm_trace::Call, + ethabi, fee::Fee, l2::L2Tx, transaction_request::PaymasterParams, vm_trace::Call, PackedEthSignature, StorageKey, Transaction, VmEvent, ACCOUNT_CODE_STORAGE_ADDRESS, CONTRACT_DEPLOYER_ADDRESS, H160, U256, }; -use zksync_utils::{h256_to_account_address, h256_to_u256, u256_to_h256}; +use zksync_utils::{bytecode::hash_bytecode, h256_to_account_address, h256_to_u256, u256_to_h256}; use crate::vm::{ db::{ZKVMData, DEFAULT_CHAIN_ID}, @@ -202,7 +198,6 @@ where /// Executes a CALL opcode on the ZK-VM. pub fn call<'a, DB, E>( call: &CallInputs, - factory_deps: Option>>, env: &'a mut Env, db: &'a mut DB, journaled_state: &'a mut JournaledState, @@ -230,7 +225,7 @@ where }, caller.to_h160(), call.transfer.value.to_u256(), - factory_deps, + None, PaymasterParams::default(), ); inspect(tx, env, db, journaled_state, ccx, msg_sender) @@ -263,7 +258,7 @@ fn inspect<'a, DB, E>( env: &'a mut Env, db: &'a mut DB, journaled_state: &'a mut JournaledState, - ccx: CheatcodeTracerContext, + mut ccx: CheatcodeTracerContext, msg_sender: Address, ) -> ZKVMResult where @@ -288,9 +283,13 @@ where } let modified_storage_keys = era_db.override_keys.clone(); - let storage_ptr = - StorageView::new(&mut era_db, modified_storage_keys, tx.common_data.initiator_address) - .into_rc_ptr(); + let storage_ptr = StorageView::new( + &mut era_db, + modified_storage_keys, + tx.common_data.initiator_address, + std::mem::take(&mut ccx.persisted_factory_deps), + ) + .into_rc_ptr(); let (tx_result, bytecodes, modified_storage) = inspect_inner( tx, storage_ptr, diff --git a/crates/zksync/core/src/vm/storage_view.rs b/crates/zksync/core/src/vm/storage_view.rs index 2968ec3a9..d5a77c1fe 100644 --- a/crates/zksync/core/src/vm/storage_view.rs +++ b/crates/zksync/core/src/vm/storage_view.rs @@ -26,6 +26,8 @@ pub(crate) struct StorageView { // Cache for `contains_key()` checks. The cache is only valid within one L1 batch execution. initial_writes_cache: HashMap, + // Simulates factory deps being persisted across tx by era + persisted_factory_deps: HashMap>, caller: H160, } @@ -35,12 +37,14 @@ impl StorageView { storage_handle: S, modified_storage_keys: HashMap, caller: H160, + persisted_factory_deps: HashMap>, ) -> Self { Self { storage_handle, modified_storage_keys, read_storage_keys: HashMap::new(), initial_writes_cache: HashMap::new(), + persisted_factory_deps, caller, } } @@ -110,7 +114,10 @@ impl ReadStorage for StorageView { } fn load_factory_dep(&mut self, hash: H256) -> Option> { - self.storage_handle.load_factory_dep(hash) + self.persisted_factory_deps + .get(&hash) + .cloned() + .or_else(|| self.storage_handle.load_factory_dep(hash)) } fn get_enumeration_index(&mut self, key: &StorageKey) -> Option { @@ -158,8 +165,12 @@ mod test { let key = StorageKey::new(account, key); let mut raw_storage = InMemoryStorage::default(); - let mut storage_view = - StorageView::new(&raw_storage, Default::default(), Default::default()); + let mut storage_view = StorageView::new( + &raw_storage, + Default::default(), + Default::default(), + Default::default(), + ); let default_value = storage_view.read_value(&key); assert_eq!(default_value, H256::zero()); @@ -170,8 +181,12 @@ mod test { 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, Default::default(), Default::default()); + let mut storage_view = StorageView::new( + &raw_storage, + Default::default(), + Default::default(), + Default::default(), + ); assert_eq!(storage_view.read_value(&key), value); assert!(!storage_view.is_write_initial(&key)); // `key` is present in `raw_storage` diff --git a/crates/zksync/core/src/vm/tracer.rs b/crates/zksync/core/src/vm/tracer.rs index 95673a761..626db2060 100644 --- a/crates/zksync/core/src/vm/tracer.rs +++ b/crates/zksync/core/src/vm/tracer.rs @@ -40,6 +40,8 @@ pub struct CheatcodeTracerContext<'a> { pub mocked_calls: HashMap>, /// Expected calls recorder. pub expected_calls: Option<&'a mut ExpectedCallTracker>, + /// Factory deps that were persisted across calls + pub persisted_factory_deps: HashMap>, } #[derive(Debug, Default)] diff --git a/zk-tests/src/Factory.t.sol b/zk-tests/src/Factory.t.sol index 6f8a9551a..64f7a4d5e 100644 --- a/zk-tests/src/Factory.t.sol +++ b/zk-tests/src/Factory.t.sol @@ -32,14 +32,13 @@ contract ZkFactoryTest is Test { assert(factory.getNumber() == 42); } - // //FIXME: fails with 'trying to decode unexisting hash' - // function testUserFactory() public { - // MyClassicFactory factory = new MyClassicFactory(); - // MyUserFactory user = new MyUserFactory(); - // user.create(address(factory), 42); - - // assert(user.getNumber(address(factory)) == 42); - // } + function testUserFactory() public { + MyClassicFactory factory = new MyClassicFactory(); + MyUserFactory user = new MyUserFactory(); + user.create(address(factory), 42); + + assert(user.getNumber(address(factory)) == 42); + } function testUserConstructorFactory() public { MyConstructorFactory factory = new MyConstructorFactory(42); From 4a747c3f3552cbc6ec3642284ea3c167eeba1c30 Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Tue, 16 Apr 2024 18:16:48 +0200 Subject: [PATCH 17/23] chore: lints --- crates/zksync/core/src/vm/runner.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/zksync/core/src/vm/runner.rs b/crates/zksync/core/src/vm/runner.rs index d8ad055f7..108a7f86e 100644 --- a/crates/zksync/core/src/vm/runner.rs +++ b/crates/zksync/core/src/vm/runner.rs @@ -45,7 +45,7 @@ use zksync_types::{ PackedEthSignature, StorageKey, Transaction, VmEvent, ACCOUNT_CODE_STORAGE_ADDRESS, CONTRACT_DEPLOYER_ADDRESS, H160, U256, }; -use zksync_utils::{bytecode::hash_bytecode, h256_to_account_address, h256_to_u256, u256_to_h256}; +use zksync_utils::{h256_to_account_address, h256_to_u256, u256_to_h256}; use crate::vm::{ db::{ZKVMData, DEFAULT_CHAIN_ID}, From b18d559428d0e6982c1464776a045d44e60b7d22 Mon Sep 17 00:00:00 2001 From: Nisheeth Barthwal Date: Tue, 16 Apr 2024 18:47:50 +0200 Subject: [PATCH 18/23] add docs --- crates/cheatcodes/src/inspector.rs | 8 +++++++- crates/zksync/core/src/vm/storage_view.rs | 13 ++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/crates/cheatcodes/src/inspector.rs b/crates/cheatcodes/src/inspector.rs index 89dc538bf..3c7d7c01e 100644 --- a/crates/cheatcodes/src/inspector.rs +++ b/crates/cheatcodes/src/inspector.rs @@ -232,7 +232,6 @@ pub struct Cheatcodes { /// Dual compiled contracts pub dual_compiled_contracts: DualCompiledContracts, - persisted_factory_deps: HashMap>, /// Logs printed during ZK-VM execution. /// EVM logs have the value `None` so they can be interpolated later, since @@ -241,6 +240,13 @@ pub struct Cheatcodes { /// Starts the cheatcode inspector in ZK mode pub startup_zk: bool, + + /// The list of factory_deps seen so far during a test or script execution. + /// Ideally these would be persisted in the storage, but since modifying [revm::JournaledState] + /// would be a significant refactor, we maintain the factory_dep part in the [Cheatcodes]. + /// This can be done as each test runs with its own [Cheatcodes] instance, thereby + /// providing the necessary level of isolation. + persisted_factory_deps: HashMap>, } impl Cheatcodes { diff --git a/crates/zksync/core/src/vm/storage_view.rs b/crates/zksync/core/src/vm/storage_view.rs index d5a77c1fe..4c18740d3 100644 --- a/crates/zksync/core/src/vm/storage_view.rs +++ b/crates/zksync/core/src/vm/storage_view.rs @@ -19,15 +19,18 @@ use crate::convert::ConvertH160; #[derive(Debug)] pub(crate) struct StorageView { pub(crate) storage_handle: S, - // Used for caching and to get the list/count of modified keys + /// Used for caching and to get the list/count of modified keys pub(crate) modified_storage_keys: HashMap, - // Used purely for caching + /// Used purely for caching pub(crate) read_storage_keys: HashMap, - // Cache for `contains_key()` checks. The cache is only valid within one L1 batch execution. + /// Cache for `contains_key()` checks. The cache is only valid within one L1 batch execution. initial_writes_cache: HashMap, - - // Simulates factory deps being persisted across tx by era + /// Simulates factory deps being persisted across transactions. + /// Since the [revm::JournaledState] storage lacks the necessary provisions + /// for storing factory_deps, we maintain them separately and pass it + /// when creating a [StorageView]. persisted_factory_deps: HashMap>, + /// The tx caller. caller: H160, } From 0396e6ba17301592caed1b66d269eca3f74cc778 Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Tue, 16 Apr 2024 21:15:59 +0200 Subject: [PATCH 19/23] fix(zk:etch): avoid `to_checked` fix(zk:factory_dep): load from storage first, then persisted deps --- crates/cheatcodes/src/inspector.rs | 8 ++------ crates/forge/bin/cmd/create.rs | 2 +- crates/zksync/core/src/cheatcodes.rs | 2 +- crates/zksync/core/src/vm/storage_view.rs | 7 +++---- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/crates/cheatcodes/src/inspector.rs b/crates/cheatcodes/src/inspector.rs index 3c7d7c01e..9f45c711d 100644 --- a/crates/cheatcodes/src/inspector.rs +++ b/crates/cheatcodes/src/inspector.rs @@ -1582,12 +1582,8 @@ impl Inspector for Cheatcodes { .unwrap_or_else(|| { panic!("failed finding contract for {:?}", call.init_code) }); - let factory_deps = self - .dual_compiled_contracts - .fetch_all_factory_deps(contract) - .into_iter() - .map(|bc| bc.to_vec()) - .collect(); + let factory_deps = + self.dual_compiled_contracts.fetch_all_factory_deps(contract); let constructor_input = call.init_code[contract.evm_bytecode.len()..].to_vec(); diff --git a/crates/forge/bin/cmd/create.rs b/crates/forge/bin/cmd/create.rs index 3aa160ce6..3c9e14fc8 100644 --- a/crates/forge/bin/cmd/create.rs +++ b/crates/forge/bin/cmd/create.rs @@ -150,7 +150,7 @@ impl CreateArgs { let mut factory_deps = dual_compiled_contracts.fetch_all_factory_deps(contract); - //for manual specified factory deps + // for manual specified factory deps for mut contract in std::mem::take(&mut self.factory_deps) { if let Some(path) = contract.path.as_mut() { *path = canonicalized(project.root().join(&path)).to_string_lossy().to_string(); diff --git a/crates/zksync/core/src/cheatcodes.rs b/crates/zksync/core/src/cheatcodes.rs index 0596054ca..57cdd6caf 100644 --- a/crates/zksync/core/src/cheatcodes.rs +++ b/crates/zksync/core/src/cheatcodes.rs @@ -149,7 +149,7 @@ pub fn etch<'a, DB>( info!(?address, bytecode = hex::encode(bytecode), "cheatcode etch"); let bytecode_hash = hash_bytecode(bytecode).to_ru256(); - let bytecode = Bytecode::new_raw(Bytes::copy_from_slice(bytecode)).to_checked(); + let bytecode = Bytecode::new_raw(Bytes::copy_from_slice(bytecode)); let account_code_addr = ACCOUNT_CODE_STORAGE_ADDRESS.to_address(); let known_codes_addr = KNOWN_CODES_STORAGE_ADDRESS.to_address(); diff --git a/crates/zksync/core/src/vm/storage_view.rs b/crates/zksync/core/src/vm/storage_view.rs index 4c18740d3..be0201da0 100644 --- a/crates/zksync/core/src/vm/storage_view.rs +++ b/crates/zksync/core/src/vm/storage_view.rs @@ -117,10 +117,9 @@ impl ReadStorage for StorageView { } fn load_factory_dep(&mut self, hash: H256) -> Option> { - self.persisted_factory_deps - .get(&hash) - .cloned() - .or_else(|| self.storage_handle.load_factory_dep(hash)) + self.storage_handle + .load_factory_dep(hash) + .or_else(|| self.persisted_factory_deps.get(&hash).cloned()) } fn get_enumeration_index(&mut self, key: &StorageKey) -> Option { From b0b1309ed468446c08b721d1325db01cddba923b Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Wed, 17 Apr 2024 16:04:11 +0200 Subject: [PATCH 20/23] refactor: persisted_factory_deps field as pub --- crates/cheatcodes/src/fs.rs | 6 +----- crates/cheatcodes/src/inspector.rs | 2 +- crates/cheatcodes/src/utils.rs | 6 +----- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/crates/cheatcodes/src/fs.rs b/crates/cheatcodes/src/fs.rs index fa635af60..3e345db94 100644 --- a/crates/cheatcodes/src/fs.rs +++ b/crates/cheatcodes/src/fs.rs @@ -382,11 +382,7 @@ mod tests { root: PathBuf::from(&env!("CARGO_MANIFEST_DIR")), ..Default::default() }; - - let mut cheatcodes = Cheatcodes::default(); - cheatcodes.config = Arc::new(config); - - cheatcodes + Cheatcodes { config: Arc::new(config), ..Default::default() } } #[test] diff --git a/crates/cheatcodes/src/inspector.rs b/crates/cheatcodes/src/inspector.rs index 9f45c711d..354167ed4 100644 --- a/crates/cheatcodes/src/inspector.rs +++ b/crates/cheatcodes/src/inspector.rs @@ -246,7 +246,7 @@ pub struct Cheatcodes { /// would be a significant refactor, we maintain the factory_dep part in the [Cheatcodes]. /// This can be done as each test runs with its own [Cheatcodes] instance, thereby /// providing the necessary level of isolation. - persisted_factory_deps: HashMap>, + pub persisted_factory_deps: HashMap>, } impl Cheatcodes { diff --git a/crates/cheatcodes/src/utils.rs b/crates/cheatcodes/src/utils.rs index 08ff7dc21..525120d7d 100644 --- a/crates/cheatcodes/src/utils.rs +++ b/crates/cheatcodes/src/utils.rs @@ -252,11 +252,7 @@ mod tests { root: PathBuf::from(&env!("CARGO_MANIFEST_DIR")), ..Default::default() }; - - let mut cheatcodes = Cheatcodes::default(); - cheatcodes.config = Arc::new(config); - - cheatcodes + Cheatcodes { config: Arc::new(config), ..Default::default() } } #[test] From ee9b7fa8134d7ad2b0a4b58121da4278e4cb7cbc Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Wed, 17 Apr 2024 16:50:19 +0200 Subject: [PATCH 21/23] fix(zk:storage): read from persisted first --- crates/zksync/core/src/vm/storage_view.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/zksync/core/src/vm/storage_view.rs b/crates/zksync/core/src/vm/storage_view.rs index be0201da0..4c18740d3 100644 --- a/crates/zksync/core/src/vm/storage_view.rs +++ b/crates/zksync/core/src/vm/storage_view.rs @@ -117,9 +117,10 @@ impl ReadStorage for StorageView { } fn load_factory_dep(&mut self, hash: H256) -> Option> { - self.storage_handle - .load_factory_dep(hash) - .or_else(|| self.persisted_factory_deps.get(&hash).cloned()) + self.persisted_factory_deps + .get(&hash) + .cloned() + .or_else(|| self.storage_handle.load_factory_dep(hash)) } fn get_enumeration_index(&mut self, key: &StorageKey) -> Option { From 615e7ebad827c59745fcfa686d57582c7bb67825 Mon Sep 17 00:00:00 2001 From: Francesco Dainese Date: Wed, 17 Apr 2024 19:10:37 +0200 Subject: [PATCH 22/23] feat(zk): `ZKVMData::with_extra_factory_deps` refactor(zk): remove persisted factory deps from `StorageView` refactor(zk:call): remove dead code --- crates/cheatcodes/src/inspector.rs | 20 ---------------- crates/zksync/core/src/vm/db.rs | 7 ++++++ crates/zksync/core/src/vm/runner.rs | 14 +++++------- crates/zksync/core/src/vm/storage_view.rs | 28 ++++------------------- 4 files changed, 18 insertions(+), 51 deletions(-) diff --git a/crates/cheatcodes/src/inspector.rs b/crates/cheatcodes/src/inspector.rs index 354167ed4..c02f79f2b 100644 --- a/crates/cheatcodes/src/inspector.rs +++ b/crates/cheatcodes/src/inspector.rs @@ -1194,26 +1194,6 @@ impl Inspector for Cheatcodes { } info!("running call in zk vm {:#?}", call); - - let code_hash = data - .journaled_state - .load_account(call.contract, data.db) - .map(|(account, _)| account.info.code_hash) - .unwrap_or_default(); - - let contract = if code_hash != KECCAK_EMPTY { - self.dual_compiled_contracts - .find_by_zk_bytecode_hash(zksync_types::H256::from(code_hash.0)) - } else { - None - }; - - if let Some(contract) = contract { - tracing::debug!(contract = contract.name, "using dual compiled contract"); - } else { - error!("no zk contract was found for {code_hash:?}"); - } - let persisted_factory_deps = self.persisted_factory_deps.clone(); let ccx = foundry_zksync_core::vm::CheatcodeTracerContext { diff --git a/crates/zksync/core/src/vm/db.rs b/crates/zksync/core/src/vm/db.rs index 1b6d0020d..1d1584763 100644 --- a/crates/zksync/core/src/vm/db.rs +++ b/crates/zksync/core/src/vm/db.rs @@ -48,6 +48,7 @@ where { /// Create a new instance of [ZKEVMData]. pub fn new(db: &'a mut DB, journaled_state: &'a mut JournaledState) -> Self { + // this will load all modified contracts and their bytecodes let mut factory_deps = journaled_state .state @@ -157,6 +158,12 @@ where account } + /// Adds the given factory deps to the internal collection + pub fn with_extra_factory_deps(mut self, extra_factory_deps: HashMap>) -> Self { + self.factory_deps.extend(extra_factory_deps); + self + } + fn read_db(&mut self, address: H160, idx: U256) -> H256 { let addr = address.to_address(); self.journaled_state.load_account(addr, self.db).expect("failed loading account"); diff --git a/crates/zksync/core/src/vm/runner.rs b/crates/zksync/core/src/vm/runner.rs index 4694b0ab3..c79e54aa7 100644 --- a/crates/zksync/core/src/vm/runner.rs +++ b/crates/zksync/core/src/vm/runner.rs @@ -287,7 +287,9 @@ where DB: Database + Send, ::Error: Debug, { - let mut era_db = ZKVMData::new_with_system_contracts(db, journaled_state); + let mut era_db = ZKVMData::new_with_system_contracts(db, journaled_state) + .with_extra_factory_deps(std::mem::take(&mut ccx.persisted_factory_deps)); + let is_create = tx.execute.contract_address == zksync_types::CONTRACT_DEPLOYER_ADDRESS; tracing::info!(?call_ctx, "executing transaction in zk vm"); @@ -305,13 +307,9 @@ where } let modified_storage_keys = era_db.override_keys.clone(); - let storage_ptr = StorageView::new( - &mut era_db, - modified_storage_keys, - tx.common_data.initiator_address, - std::mem::take(&mut ccx.persisted_factory_deps), - ) - .into_rc_ptr(); + let storage_ptr = + StorageView::new(&mut era_db, modified_storage_keys, tx.common_data.initiator_address) + .into_rc_ptr(); let (tx_result, bytecodes, modified_storage) = inspect_inner( tx, storage_ptr, diff --git a/crates/zksync/core/src/vm/storage_view.rs b/crates/zksync/core/src/vm/storage_view.rs index 4c18740d3..7bcfea892 100644 --- a/crates/zksync/core/src/vm/storage_view.rs +++ b/crates/zksync/core/src/vm/storage_view.rs @@ -25,11 +25,6 @@ pub(crate) struct StorageView { pub(crate) read_storage_keys: HashMap, /// Cache for `contains_key()` checks. The cache is only valid within one L1 batch execution. initial_writes_cache: HashMap, - /// Simulates factory deps being persisted across transactions. - /// Since the [revm::JournaledState] storage lacks the necessary provisions - /// for storing factory_deps, we maintain them separately and pass it - /// when creating a [StorageView]. - persisted_factory_deps: HashMap>, /// The tx caller. caller: H160, } @@ -40,14 +35,12 @@ impl StorageView { storage_handle: S, modified_storage_keys: HashMap, caller: H160, - persisted_factory_deps: HashMap>, ) -> Self { Self { storage_handle, modified_storage_keys, read_storage_keys: HashMap::new(), initial_writes_cache: HashMap::new(), - persisted_factory_deps, caller, } } @@ -117,10 +110,7 @@ impl ReadStorage for StorageView { } fn load_factory_dep(&mut self, hash: H256) -> Option> { - self.persisted_factory_deps - .get(&hash) - .cloned() - .or_else(|| self.storage_handle.load_factory_dep(hash)) + self.storage_handle.load_factory_dep(hash) } fn get_enumeration_index(&mut self, key: &StorageKey) -> Option { @@ -168,12 +158,8 @@ mod test { let key = StorageKey::new(account, key); let mut raw_storage = InMemoryStorage::default(); - let mut storage_view = StorageView::new( - &raw_storage, - Default::default(), - Default::default(), - Default::default(), - ); + let mut storage_view = + StorageView::new(&raw_storage, Default::default(), Default::default()); let default_value = storage_view.read_value(&key); assert_eq!(default_value, H256::zero()); @@ -184,12 +170,8 @@ mod test { 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, - Default::default(), - Default::default(), - Default::default(), - ); + let mut storage_view = + StorageView::new(&raw_storage, Default::default(), Default::default()); assert_eq!(storage_view.read_value(&key), value); assert!(!storage_view.is_write_initial(&key)); // `key` is present in `raw_storage` From dcc0f915924bedf1f00a1a0af7219ed12e3e7405 Mon Sep 17 00:00:00 2001 From: Karrq Date: Thu, 18 Apr 2024 09:35:55 +0200 Subject: [PATCH 23/23] docs(zk): factory deps explanations Co-authored-by: Nisheeth Barthwal --- crates/zksync/core/src/vm/db.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/zksync/core/src/vm/db.rs b/crates/zksync/core/src/vm/db.rs index 1d1584763..385c761d8 100644 --- a/crates/zksync/core/src/vm/db.rs +++ b/crates/zksync/core/src/vm/db.rs @@ -48,7 +48,7 @@ where { /// Create a new instance of [ZKEVMData]. pub fn new(db: &'a mut DB, journaled_state: &'a mut JournaledState) -> Self { - // this will load all modified contracts and their bytecodes + // load all deployed contract bytecodes from the JournaledState as factory deps let mut factory_deps = journaled_state .state @@ -158,7 +158,7 @@ where account } - /// Adds the given factory deps to the internal collection + /// Extends the currently known factory deps with the provided input pub fn with_extra_factory_deps(mut self, extra_factory_deps: HashMap>) -> Self { self.factory_deps.extend(extra_factory_deps); self