Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: fetch all factory deps for a given contract #316

Merged
merged 27 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
1873d96
fix(compile:zk): use factory deps from compilation
Karrq Apr 9, 2024
e7a1264
algo
Apr 9, 2024
8631f6c
feat(create:zk): fetch all unique factory deps
Karrq Apr 9, 2024
4ce160c
move fetch factory deps to FindContract trait
Apr 10, 2024
75f82b8
fix(zkvm:broadcast): fetch all factory_deps
Karrq Apr 10, 2024
36f32f7
fix(zkvm:create): fetch all factory deps
Karrq Apr 10, 2024
96bcb0e
chore: cleanup & todos
Karrq Apr 11, 2024
44a5778
tests(zk): add `Factory` family of tests
Karrq Apr 11, 2024
9d44a95
refactor: dedicated `DualCompiledContracts`
Karrq Apr 12, 2024
78199a4
fix(test:zk): Factory contract cleanup
Karrq Apr 12, 2024
5a1a8b3
fix(call:zk): pass factory deps
Karrq Apr 12, 2024
b776171
chore: fmt
Karrq Apr 12, 2024
b274948
fix(zk:create): restore handle `--factory-deps`
Karrq Apr 12, 2024
fdcf193
refactory(test:zk): split Factory tests & sources
Karrq Apr 15, 2024
0cd560c
Merge branch 'dev' into fix/nested-factory-deps
HermanObst Apr 15, 2024
e7cbe94
chore(docs): formatting
Karrq Apr 16, 2024
57a8720
feat(zkvm): persist seen factory deps
Karrq Apr 16, 2024
4a747c3
chore: lints
Karrq Apr 16, 2024
b18d559
add docs
nbaztec Apr 16, 2024
0396e6b
fix(zk:etch): avoid `to_checked`
Karrq Apr 16, 2024
58521b3
Merge remote-tracking branch 'origin/dev' into fix/nested-factory-deps
Karrq Apr 16, 2024
b0b1309
refactor: persisted_factory_deps field as pub
Karrq Apr 17, 2024
ee9b7fa
fix(zk:storage): read from persisted first
Karrq Apr 17, 2024
615e7eb
feat(zk): `ZKVMData::with_extra_factory_deps`
Karrq Apr 17, 2024
4c8b5fd
Merge branch 'dev' into fix/nested-factory-deps
Karrq Apr 17, 2024
dcc0f91
docs(zk): factory deps explanations
Karrq Apr 18, 2024
81616fa
Merge branch 'dev' into fix/nested-factory-deps
HermanObst Apr 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/cheatcodes/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -42,7 +42,7 @@ pub struct CheatsConfig {
/// Script wallets
pub script_wallets: Option<ScriptWallets>,
/// ZKSolc -> Solc Contract codes
pub dual_compiled_contracts: Vec<DualCompiledContract>,
pub dual_compiled_contracts: DualCompiledContracts,
/// Use ZK-VM on startup
pub use_zk: bool,
}
Expand All @@ -53,7 +53,7 @@ impl CheatsConfig {
config: &Config,
evm_opts: EvmOpts,
script_wallets: Option<ScriptWallets>,
dual_compiled_contracts: Vec<DualCompiledContract>,
dual_compiled_contracts: DualCompiledContracts,
use_zk: bool,
) -> Self {
let mut allowed_paths = vec![config.__root.0.clone()];
Expand Down
57 changes: 29 additions & 28 deletions crates/cheatcodes/src/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ 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,
hash_bytecode, ZkTransactionMetadata,
};
use itertools::Itertools;
use revm::{
Expand All @@ -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,
};

Expand Down Expand Up @@ -231,7 +231,7 @@ pub struct Cheatcodes {
pub use_zk_vm: bool,

/// Dual compiled contracts
pub dual_compiled_contracts: Vec<DualCompiledContract>,
pub dual_compiled_contracts: DualCompiledContracts,

/// Logs printed during ZK-VM execution.
/// EVM logs have the value `None` so they can be interpolated later, since
Expand All @@ -240,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.
pub persisted_factory_deps: HashMap<H256, Vec<u8>>,
}

impl Cheatcodes {
Expand Down Expand Up @@ -404,10 +411,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,
Expand Down Expand Up @@ -1189,31 +1194,15 @@ impl<DB: DatabaseExt + Send> Inspector<DB> 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_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 {
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,
contract,
data.env,
data.db,
&mut data.journaled_state,
Expand Down Expand Up @@ -1569,10 +1558,13 @@ impl<DB: DatabaseExt + Send> Inspector<DB> 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)
});
let factory_deps =
self.dual_compiled_contracts.fetch_all_factory_deps(contract);

let constructor_input =
call.init_code[contract.evm_bytecode.len()..].to_vec();
let create_input = foundry_zksync_core::encode_create_params(
Expand All @@ -1581,7 +1573,6 @@ impl<DB: DatabaseExt + Send> Inspector<DB> for Cheatcodes {
constructor_input,
);
bytecode = Bytes::from(create_input);
let factory_deps = vec![contract.zk_deployed_bytecode.clone()];

Some(ZkTransactionMetadata { factory_deps })
} else {
Expand Down Expand Up @@ -1674,17 +1665,27 @@ impl<DB: DatabaseExt + Send> Inspector<DB> 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);

// 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())));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a follow up, we should try to remove this part, and perhaps extend it only with the bytecodes returned from the era vm. So in theory, every known bytecode will reside here. Or perhaps we can also think if it's easier to support it directly in the Backend and via some changes to DatabaseExt to actually store the factory_deps directly in the storage. But that all can be a later refactor.


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,
zk_contract,
factory_deps,
data.env,
data.db,
&mut data.journaled_state,
Expand Down
2 changes: 2 additions & 0 deletions crates/cheatcodes/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ 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![],
Karrq marked this conversation as resolved.
Show resolved Hide resolved
evm_bytecode_hash: *evmBytecodeHash,
evm_deployed_bytecode: evmDeployedBytecode.clone(),
evm_bytecode: evmBytecode.clone(),
Expand Down
53 changes: 29 additions & 24 deletions crates/forge/bin/cmd/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ 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};

Expand Down Expand Up @@ -123,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
Expand All @@ -136,11 +134,12 @@ 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
))?;

let zk_bin = CompactBytecode {
object: BytecodeObject::Bytecode(Bytes::from(
contract.zk_deployed_bytecode.clone(),
Expand All @@ -149,8 +148,9 @@ impl CreateArgs {
source_map: Default::default(),
};

let mut factory_deps = Vec::with_capacity(self.factory_deps.len());
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) {
Karrq marked this conversation as resolved.
Show resolved Hide resolved
if let Some(path) = contract.path.as_mut() {
*path = canonicalized(project.root().join(&path)).to_string_lossy().to_string();
Expand All @@ -163,16 +163,27 @@ impl CreateArgs {
let zk = 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 {}",
contract.name
))?;

factory_deps.push(zk.zk_deployed_bytecode.clone());
// 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) {
let additional_factory_deps =
dual_compiled_contracts.fetch_all_factory_deps(zk);
factory_deps.extend(additional_factory_deps);
factory_deps.dedup();
}
}

(abi, zk_bin, Some((contract, factory_deps)))
(
abi,
zk_bin,
Some((contract, factory_deps.into_iter().map(|bc| bc.to_vec()).collect())),
)
} else {
(abi, bin, None)
};
Expand Down Expand Up @@ -297,16 +308,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 {
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(factory_deps.clone()))
} else {
Expand All @@ -328,9 +330,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);

Expand Down Expand Up @@ -358,9 +360,12 @@ impl CreateArgs {
.tx
.set_to(NameOrAddress::from(foundry_zksync_core::CONTRACT_DEPLOYER_ADDRESS));

let estimated_gas =
foundry_zksync_core::estimate_gas(&deployer.tx, factory_deps, &provider)
.await?;
let estimated_gas = foundry_zksync_core::estimate_gas(
&deployer.tx,
factory_deps.clone(),
&provider,
)
.await?;
deployer.tx.set_gas(estimated_gas.limit.to_ethers());
deployer.tx.set_gas_price(estimated_gas.price.to_ethers());
}
Expand Down
8 changes: 4 additions & 4 deletions crates/forge/bin/cmd/script/broadcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -317,7 +317,7 @@ impl ScriptArgs {
mut script_config: ScriptConfig,
verify: VerifyBundle,
signers: &HashMap<Address, WalletSigner>,
dual_compiled_contracts: Option<Vec<DualCompiledContract>>,
dual_compiled_contracts: Option<DualCompiledContracts>,
) -> Result<()> {
if let Some(txs) = result.transactions.take() {
script_config.collect_rpcs(&txs);
Expand Down Expand Up @@ -419,7 +419,7 @@ impl ScriptArgs {
script_config: &mut ScriptConfig,
decoder: &CallTraceDecoder,
known_contracts: &ContractsByArtifact,
dual_compiled_contracts: Option<Vec<DualCompiledContract>>,
dual_compiled_contracts: Option<DualCompiledContracts>,
) -> Result<Vec<ScriptSequence>> {
if !txs.is_empty() {
let gas_filled_txs = self
Expand Down Expand Up @@ -458,7 +458,7 @@ impl ScriptArgs {
script_config: &ScriptConfig,
decoder: &CallTraceDecoder,
known_contracts: &ContractsByArtifact,
dual_compiled_contracts: Option<Vec<DualCompiledContract>>,
dual_compiled_contracts: Option<DualCompiledContracts>,
) -> Result<VecDeque<TransactionWithMetadata>> {
let gas_filled_txs = if self.skip_simulation {
shell::println("\nSKIPPING ON CHAIN SIMULATION.")?;
Expand Down
6 changes: 3 additions & 3 deletions crates/forge/bin/cmd/script/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -228,5 +228,5 @@ pub struct BuildOutput {
pub libraries: Libraries,
pub predeploy_libraries: Vec<Bytes>,
pub sources: ContractSources,
pub dual_compiled_contracts: Option<Vec<DualCompiledContract>>,
pub dual_compiled_contracts: Option<DualCompiledContracts>,
}
6 changes: 3 additions & 3 deletions crates/forge/bin/cmd/script/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -159,7 +159,7 @@ impl ScriptArgs {
predeploy_libraries: Vec<Bytes>,
result: &mut ScriptResult,
script_wallets: ScriptWallets,
dual_compiled_contracts: Option<Vec<DualCompiledContract>>,
dual_compiled_contracts: Option<DualCompiledContracts>,
) -> Result<Option<NewSenderChanges>> {
if let Some(new_sender) = self.maybe_new_sender(
&script_config.evm_opts,
Expand Down Expand Up @@ -332,7 +332,7 @@ impl ScriptArgs {
first_run_result: &mut ScriptResult,
linker: Linker,
script_wallets: ScriptWallets,
dual_compiled_contracts: Option<Vec<DualCompiledContract>>,
dual_compiled_contracts: Option<DualCompiledContracts>,
) -> Result<(Libraries, ArtifactContracts<ContractBytecodeSome>)> {
// if we had a new sender that requires relinking, we need to
// get the nonce mainnet for accurate addresses for predeploy libs
Expand Down
Loading
Loading