From ab00f2ea4a6bd2100b7fdb5f16687fd078a1f6f3 Mon Sep 17 00:00:00 2001 From: Karrq Date: Wed, 3 Jul 2024 22:18:09 +0200 Subject: [PATCH] fix: batch factory deps for broadcastable tx (#455) --- crates/cheatcodes/src/inspector.rs | 42 +++++++++++++++++-- crates/evm/core/src/backend/fuzz.rs | 3 +- crates/evm/core/src/backend/mod.rs | 3 +- crates/evm/evm/src/executors/mod.rs | 36 +++++++++++++++-- crates/forge/bin/cmd/script/broadcast.rs | 2 +- crates/forge/bin/cmd/script/executor.rs | 2 +- crates/zksync/core/src/cheatcodes.rs | 13 ++++-- crates/zksync/core/src/lib.rs | 2 +- crates/zksync/core/src/vm/inspect.rs | 11 ++++- crates/zksync/core/src/vm/mod.rs | 4 +- crates/zksync/core/src/vm/runner.rs | 15 +++++-- crates/zksync/core/src/vm/tracer.rs | 2 + zk-tests/script/Deploy.s.sol | 45 +++++---------------- zk-tests/src/Contracts.t.sol | 29 +++++++++++++ zk-tests/src/Greeter.sol | 38 +++++++++++++++++ zk-tests/src/LargeFactoryDependencies.t.sol | 10 ++++- zk-tests/test.sh | 3 ++ 17 files changed, 199 insertions(+), 61 deletions(-) create mode 100644 zk-tests/src/Greeter.sol diff --git a/crates/cheatcodes/src/inspector.rs b/crates/cheatcodes/src/inspector.rs index d0c026033..c1e8d3a9d 100644 --- a/crates/cheatcodes/src/inspector.rs +++ b/crates/cheatcodes/src/inspector.rs @@ -1591,7 +1591,7 @@ impl Inspector for Cheatcodes { ); let is_fixed_gas_limit = check_if_fixed_gas_limit(data, call.gas_limit); - let zk_tx = if self.use_zk_vm { + let mut zk_tx = if self.use_zk_vm { to = Some(CONTRACT_DEPLOYER_ADDRESS.to_address()); nonce = foundry_zksync_core::nonce( broadcast.new_origin, @@ -1617,13 +1617,46 @@ impl Inspector for Cheatcodes { ); bytecode = Bytes::from(create_input); - Some(ZkTransactionMetadata { factory_deps }) + Some(factory_deps) } else { None }; + let rpc = data.db.active_fork_url(); + + if let Some(factory_deps) = zk_tx { + let mut batched = + foundry_zksync_core::vm::batch_factory_dependencies(factory_deps); + debug!(batches = batched.len(), "splitting factory deps for broadcast"); + // the last batch is the final one that does the deployment + zk_tx = batched.pop(); + + for factory_deps in batched { + self.broadcastable_transactions.push_back(BroadcastableTransaction { + rpc: rpc.clone(), + transaction: TransactionRequest { + from: Some(broadcast.new_origin), + to: Some(Address::ZERO), + value: Some(call.value), + input: TransactionInput::default(), + nonce: Some(U64::from(nonce)), + gas: if is_fixed_gas_limit { + Some(U256::from(call.gas_limit)) + } else { + None + }, + ..Default::default() + }, + zk_tx: Some(ZkTransactionMetadata { factory_deps }), + }); + + //update nonce for each tx + nonce += 1; + } + } + self.broadcastable_transactions.push_back(BroadcastableTransaction { - rpc: data.db.active_fork_url(), + rpc: rpc.clone(), transaction: TransactionRequest { from: Some(broadcast.new_origin), to, @@ -1637,8 +1670,9 @@ impl Inspector for Cheatcodes { }, ..Default::default() }, - zk_tx, + zk_tx: zk_tx.map(|factory_deps| ZkTransactionMetadata { factory_deps }), }); + let kind = match call.scheme { CreateScheme::Create => "create", CreateScheme::Create2 { .. } => "create2", diff --git a/crates/evm/core/src/backend/fuzz.rs b/crates/evm/core/src/backend/fuzz.rs index e3b3c5d7e..6b4ff2727 100644 --- a/crates/evm/core/src/backend/fuzz.rs +++ b/crates/evm/core/src/backend/fuzz.rs @@ -53,13 +53,14 @@ impl<'a> FuzzBackendWrapper<'a> { pub fn inspect_ref_zk( &mut self, env: &mut Env, + persisted_factory_deps: &mut HashMap>, factory_deps: Option>>, ) -> eyre::Result { // this is a new call to inspect with a new env, so even if we've cloned the backend // already, we reset the initialized state self.is_initialized = false; - foundry_zksync_core::vm::transact(factory_deps, env, self) + foundry_zksync_core::vm::transact(Some(persisted_factory_deps), factory_deps, env, self) } /// Executes the configured transaction of the `env` without committing state changes diff --git a/crates/evm/core/src/backend/mod.rs b/crates/evm/core/src/backend/mod.rs index 699e459eb..d87425f5e 100644 --- a/crates/evm/core/src/backend/mod.rs +++ b/crates/evm/core/src/backend/mod.rs @@ -840,11 +840,12 @@ impl Backend { pub fn inspect_ref_zk( &mut self, env: &mut Env, + persisted_factory_deps: &mut HashMap>, factory_deps: Option>>, ) -> eyre::Result { self.initialize(env); - foundry_zksync_core::vm::transact(factory_deps, env, self) + foundry_zksync_core::vm::transact(Some(persisted_factory_deps), factory_deps, env, self) } /// Returns true if the address is a precompile diff --git a/crates/evm/evm/src/executors/mod.rs b/crates/evm/evm/src/executors/mod.rs index e3c83e852..9f75aae55 100644 --- a/crates/evm/evm/src/executors/mod.rs +++ b/crates/evm/evm/src/executors/mod.rs @@ -75,6 +75,8 @@ pub struct Executor { /// Sets up the next transaction to be executed as a ZK transaction. zk_tx: Option, + // simulate persisted factory deps + zk_persisted_factory_deps: HashMap>, pub use_zk: bool, } @@ -92,7 +94,15 @@ impl Executor { }, ); - Executor { backend, env, inspector, gas_limit, zk_tx: None, use_zk: false } + Executor { + backend, + env, + inspector, + gas_limit, + zk_tx: None, + zk_persisted_factory_deps: Default::default(), + use_zk: false, + } } /// Creates the default CREATE2 Contract Deployer for local tests and scripts. @@ -324,7 +334,11 @@ impl Executor { let mut db = FuzzBackendWrapper::new(&self.backend); let result = match &self.zk_tx { - Some(zk_tx) => db.inspect_ref_zk(&mut env, Some(zk_tx.factory_deps.clone()))?, + Some(zk_tx) => db.inspect_ref_zk( + &mut env, + &mut self.zk_persisted_factory_deps.clone(), + Some(zk_tx.factory_deps.clone()), + )?, None => db.inspect_ref(&mut env, &mut inspector)?, }; @@ -345,8 +359,12 @@ impl Executor { // execute the call let mut inspector = self.inspector.clone(); - let result = match self.zk_tx.take() { - Some(zk_tx) => self.backend.inspect_ref_zk(&mut env, Some(zk_tx.factory_deps))?, + let result = match &self.zk_tx { + Some(zk_tx) => self.backend.inspect_ref_zk( + &mut env, + &mut self.zk_persisted_factory_deps.clone(), + Some(zk_tx.factory_deps.clone()), + )?, None => self.backend.inspect_ref(&mut env, &mut inspector)?, }; @@ -356,6 +374,16 @@ impl Executor { /// Commit the changeset to the database and adjust `self.inspector_config` /// values according to the executed call result fn commit(&mut self, result: &mut RawCallResult) { + // Persist factory deps from just executed tx + if let Some(zk_tx) = self.zk_tx.take() { + self.zk_persisted_factory_deps.extend( + zk_tx + .factory_deps + .into_iter() + .map(|dep| (foundry_zksync_core::hash_bytecode(&dep), dep)), + ); + } + // Persist changes to db if let Some(changes) = &result.state_changeset { self.backend.commit(changes.clone()); diff --git a/crates/forge/bin/cmd/script/broadcast.rs b/crates/forge/bin/cmd/script/broadcast.rs index 9480e000d..f00e9467c 100644 --- a/crates/forge/bin/cmd/script/broadcast.rs +++ b/crates/forge/bin/cmd/script/broadcast.rs @@ -661,7 +661,7 @@ impl ScriptArgs { .nonce(legacy_or_1559.nonce().unwrap()) .gas_price(legacy_or_1559.gas_price().unwrap()) .max_fee_per_gas(legacy_or_1559.max_cost().unwrap()) - .data(legacy_or_1559.data().cloned().unwrap()) + .data(legacy_or_1559.data().cloned().unwrap_or_default()) .custom_data(custom_data); let gas_price = provider.get_gas_price().await?; diff --git a/crates/forge/bin/cmd/script/executor.rs b/crates/forge/bin/cmd/script/executor.rs index c89b78919..df9472e6e 100644 --- a/crates/forge/bin/cmd/script/executor.rs +++ b/crates/forge/bin/cmd/script/executor.rs @@ -106,7 +106,7 @@ impl ScriptArgs { contracts: &ContractsByArtifact, dual_compiled_contracts: Option, ) -> Result> { - trace!(target: "script", "executing onchain simulation"); + trace!(target: "script", ?transactions, "executing onchain simulation"); let runners = Arc::new( self.build_runners(script_config, dual_compiled_contracts) diff --git a/crates/zksync/core/src/cheatcodes.rs b/crates/zksync/core/src/cheatcodes.rs index 7bc5f7029..4647be977 100644 --- a/crates/zksync/core/src/cheatcodes.rs +++ b/crates/zksync/core/src/cheatcodes.rs @@ -9,7 +9,7 @@ use tracing::info; use zksync_types::{ block::{pack_block_info, unpack_block_info}, get_nonce_key, - utils::storage_key_for_eth_balance, + utils::{decompose_full_nonce, storage_key_for_eth_balance}, ACCOUNT_CODE_STORAGE_ADDRESS, CURRENT_VIRTUAL_BLOCK_INFO_POSITION, KNOWN_CODES_STORAGE_ADDRESS, L2_BASE_TOKEN_ADDRESS, NONCE_HOLDER_ADDRESS, SYSTEM_CONTEXT_ADDRESS, }; @@ -109,13 +109,17 @@ pub fn set_nonce<'a, DB>( ::Error: Debug, { info!(?address, ?nonce, "cheatcode setNonce"); + //ensure nonce is _only_ tx nonce + let (tx_nonce, _deploy_nonce) = decompose_full_nonce(nonce.to_u256()); let nonce_addr = NONCE_HOLDER_ADDRESS.to_address(); journaled_state.load_account(nonce_addr, db).expect("account could not be loaded"); let zk_address = address.to_h160(); let nonce_key = get_nonce_key(&zk_address).key().to_ru256(); journaled_state.touch(&nonce_addr); - journaled_state.sstore(nonce_addr, nonce_key, nonce, db).expect("failed storing value"); + journaled_state + .sstore(nonce_addr, nonce_key, tx_nonce.to_ru256(), db) + .expect("failed storing value"); } /// Gets nonce for a specific address. @@ -134,9 +138,10 @@ where journaled_state.load_account(nonce_addr, db).expect("account could not be loaded"); let zk_address = address.to_h160(); let nonce_key = get_nonce_key(&zk_address).key().to_ru256(); - let (nonce, _) = journaled_state.sload(nonce_addr, nonce_key, db).unwrap_or_default(); + let (full_nonce, _) = journaled_state.sload(nonce_addr, nonce_key, db).unwrap_or_default(); - nonce + let (tx_nonce, _deploy_nonce) = decompose_full_nonce(full_nonce.to_u256()); + tx_nonce.to_ru256() } /// Sets code for a specific address. diff --git a/crates/zksync/core/src/lib.rs b/crates/zksync/core/src/lib.rs index a1c028bba..990bb448f 100644 --- a/crates/zksync/core/src/lib.rs +++ b/crates/zksync/core/src/lib.rs @@ -26,7 +26,7 @@ pub use vm::{balance, encode_create_params, nonce}; use zksync_types::utils::storage_key_for_eth_balance; pub use zksync_types::{ - ACCOUNT_CODE_STORAGE_ADDRESS, CONTRACT_DEPLOYER_ADDRESS, L2_BASE_TOKEN_ADDRESS, + ACCOUNT_CODE_STORAGE_ADDRESS, CONTRACT_DEPLOYER_ADDRESS, H256, L2_BASE_TOKEN_ADDRESS, NONCE_HOLDER_ADDRESS, }; pub use zksync_utils::bytecode::hash_bytecode; diff --git a/crates/zksync/core/src/vm/inspect.rs b/crates/zksync/core/src/vm/inspect.rs index 98fe887c1..70f20aa64 100644 --- a/crates/zksync/core/src/vm/inspect.rs +++ b/crates/zksync/core/src/vm/inspect.rs @@ -349,6 +349,7 @@ fn inspect_inner( expected_calls.insert(*addr, v.clone()); } } + let is_static = call_ctx.is_static; let tracers = vec![ CallTracer::new(call_tracer_result.clone()).into_tracer_pointer(), CheatcodeTracer::new( @@ -414,7 +415,13 @@ fn inspect_inner( .iter() .map(|b| bytecode_to_factory_dep(b.original.clone())) .collect(); - let modified_keys = storage.borrow().modified_storage_keys().clone(); + + let modified_keys = if is_static { + Default::default() + } else { + storage.borrow().modified_storage_keys().clone() + }; + (tx_result, bytecodes, modified_keys) } @@ -505,7 +512,7 @@ pub const MAX_FACTORY_DEPENDENCIES_SIZE_BYTES: usize = 100000; // 100kB /// For large factory_deps the VM can run out of gas. To avoid this case we batch factory_deps /// on the basis of [MAX_FACTORY_DEPENDENCIES_SIZE_BYTES] and deploy all but the last batch /// via empty transactions, with the last one deployed normally via create. -fn batch_factory_dependencies(mut factory_deps: Vec>) -> Vec>> { +pub fn batch_factory_dependencies(mut factory_deps: Vec>) -> Vec>> { let factory_deps_count = factory_deps.len(); let factory_deps_sizes = factory_deps.iter().map(|dep| dep.len()).collect_vec(); let factory_deps_total_size = factory_deps_sizes.iter().sum::(); diff --git a/crates/zksync/core/src/vm/mod.rs b/crates/zksync/core/src/vm/mod.rs index eddfb0426..a6cafac57 100644 --- a/crates/zksync/core/src/vm/mod.rs +++ b/crates/zksync/core/src/vm/mod.rs @@ -6,6 +6,8 @@ mod runner; mod storage_view; mod tracer; -pub use inspect::{inspect, inspect_as_batch, ZKVMExecutionResult, ZKVMResult}; +pub use inspect::{ + batch_factory_dependencies, inspect, inspect_as_batch, ZKVMExecutionResult, ZKVMResult, +}; pub use runner::{balance, call, code_hash, create, encode_create_params, nonce, transact}; pub use tracer::CheatcodeTracerContext; diff --git a/crates/zksync/core/src/vm/runner.rs b/crates/zksync/core/src/vm/runner.rs index b2f105ec3..124b86403 100644 --- a/crates/zksync/core/src/vm/runner.rs +++ b/crates/zksync/core/src/vm/runner.rs @@ -14,7 +14,7 @@ use zksync_types::{ U256, }; -use std::{cmp::min, fmt::Debug}; +use std::{cmp::min, collections::HashMap, fmt::Debug}; use crate::{ convert::{ConvertAddress, ConvertH160, ConvertRU256, ConvertU256}, @@ -28,6 +28,7 @@ use crate::{ /// Transacts pub fn transact<'a, DB>( + persisted_factory_deps: Option<&'a mut HashMap>>, factory_deps: Option>>, env: &'a mut Env, db: &'a mut DB, @@ -36,7 +37,7 @@ where DB: Database + Send, ::Error: Debug, { - debug!("zk transact"); + debug!(calldata = ?env.tx.data, fdeps = factory_deps.as_ref().map(|v| v.len()).unwrap_or_default(), "zk transact"); let mut journaled_state = JournaledState::new( env.cfg.spec_id, Precompiles::new(to_precompile_id(env.cfg.spec_id)) @@ -55,7 +56,7 @@ where }; let (gas_limit, max_fee_per_gas) = gas_params(env, db, &mut journaled_state, caller); - info!(?gas_limit, ?max_fee_per_gas, "tx gas parameters"); + debug!(?gas_limit, ?max_fee_per_gas, "tx gas parameters"); let tx = L2Tx::new( transact_to, env.tx.data.to_vec(), @@ -81,14 +82,18 @@ where block_timestamp: env.block.timestamp, block_basefee: min(max_fee_per_gas.to_ru256(), env.block.basefee), is_create, + is_static: false, }; + let mut cheatcode_tracer_context = + CheatcodeTracerContext { persisted_factory_deps, ..Default::default() }; + match inspect::<_, DB::Error>( tx, env, db, &mut journaled_state, - &mut Default::default(), + &mut cheatcode_tracer_context, call_ctx, ) { Ok(ZKVMExecutionResult { execution_result: result, .. }) => { @@ -187,6 +192,7 @@ where block_timestamp: env.block.timestamp, block_basefee: min(max_fee_per_gas.to_ru256(), env.block.basefee), is_create: true, + is_static: false, }; inspect_as_batch(tx, env, db, journaled_state, &mut ccx, call_ctx) @@ -242,6 +248,7 @@ where block_timestamp: env.block.timestamp, block_basefee: min(max_fee_per_gas.to_ru256(), env.block.basefee), is_create: false, + is_static: call.is_static, }; inspect(tx, env, db, journaled_state, &mut ccx, call_ctx) diff --git a/crates/zksync/core/src/vm/tracer.rs b/crates/zksync/core/src/vm/tracer.rs index f5b0011a8..8dbf12280 100644 --- a/crates/zksync/core/src/vm/tracer.rs +++ b/crates/zksync/core/src/vm/tracer.rs @@ -107,6 +107,8 @@ pub struct CallContext { pub block_basefee: rU256, /// Whether the current call is a create. pub is_create: bool, + /// Whether the current call is a static call. + pub is_static: bool, } /// A tracer to allow for foundry-specific functionality. diff --git a/zk-tests/script/Deploy.s.sol b/zk-tests/script/Deploy.s.sol index 6188f70d1..10dc95c8a 100644 --- a/zk-tests/script/Deploy.s.sol +++ b/zk-tests/script/Deploy.s.sol @@ -2,41 +2,7 @@ pragma solidity ^0.8.18; import {Script} from "forge-std/Script.sol"; -import {Test} from "forge-std/Test.sol"; -import {console2 as console} from "forge-std/console2.sol"; - -contract Greeter { - string name; - uint256 age; - - event Greet(string greet); - - function greeting(string memory _name) public returns (string memory) { - name = _name; - string memory greet = string(abi.encodePacked("Hello ", _name)); - emit Greet(greet); - return greet; - } - - function greeting2( - string memory _name, - uint256 n - ) public returns (uint256) { - name = _name; - string memory greet = string(abi.encodePacked("Hello ", _name)); - console.log(name); - emit Greet(greet); - return n * 2; - } - - function setAge(uint256 _age) public { - age = _age; - } - - function getAge() public view returns (uint256) { - return age; - } -} +import {Greeter} from "../src/Greeter.sol"; contract DeployScript is Script { // Vm constant vm = Vm(HEVM_ADDRESS); @@ -51,9 +17,16 @@ contract DeployScript is Script { ); require(success, "zkVm() call failed"); vm.startBroadcast(); + greeter = new Greeter(); - greeter.greeting("john"); + greeter.setAge(123); + uint256 age = greeter.getAge(); + + greeter.greeting("john"); + vm.stopBroadcast(); + + assert(age == 123); } } diff --git a/zk-tests/src/Contracts.t.sol b/zk-tests/src/Contracts.t.sol index 6dba9686d..69b08cabd 100644 --- a/zk-tests/src/Contracts.t.sol +++ b/zk-tests/src/Contracts.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.13; import {Test, console2 as console} from "forge-std/Test.sol"; import {ConstantNumber} from "./ConstantNumber.sol"; +import {Greeter} from "./Greeter.sol"; interface ISystemContractDeployer { function getNewAddressCreate2( @@ -300,4 +301,32 @@ contract ZkContractsTest is Test { assertEq(42, multiNumber.one()); assertEq(2, multiNumber.two()); } + + function testZkStaticCalls() public { + (bool success, ) = address(vm).call( + abi.encodeWithSignature("zkVm(bool)", true) + ); + require(success, "zkVm() call failed"); + address sender = address(this); + uint64 startingNonce = vm.getNonce(sender); + + //this ensures calls & deployments increase the nonce + vm.startBroadcast(sender); + + Greeter greeter = new Greeter(); + assert(vm.getNonce(sender) == startingNonce + 1); + + greeter.setAge(42); + assert(vm.getNonce(sender) == startingNonce + 2); + + // static-call, nonce shouldn't change + uint256 age = greeter.getAge(); + assert(age == 42); + assert(vm.getNonce(sender) == startingNonce + 2); + + uint256 num = greeter.greeting2("zksync", 2); + assert(num == 4); + assert(vm.getNonce(sender) == startingNonce + 3); + vm.stopBroadcast(); + } } diff --git a/zk-tests/src/Greeter.sol b/zk-tests/src/Greeter.sol new file mode 100644 index 000000000..0989f53e6 --- /dev/null +++ b/zk-tests/src/Greeter.sol @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 +pragma solidity ^0.8.18; + +contract Greeter { + string name; + uint256 age; + + event Greet(string greet); + + function greeting(string memory _name) public returns (string memory) { + name = _name; + + string memory greet = string(abi.encodePacked("Hello ", _name)); + emit Greet(greet); + + return greet; + } + + function greeting2( + string memory _name, + uint256 n + ) public returns (uint256) { + name = _name; + + string memory greet = string(abi.encodePacked("Hello ", _name)); + emit Greet(greet); + + return n * 2; + } + + function setAge(uint256 _age) public { + age = _age; + } + + function getAge() public view returns (uint256) { + return age; + } +} diff --git a/zk-tests/src/LargeFactoryDependencies.t.sol b/zk-tests/src/LargeFactoryDependencies.t.sol index 72c06fdb7..0af9b6654 100644 --- a/zk-tests/src/LargeFactoryDependencies.t.sol +++ b/zk-tests/src/LargeFactoryDependencies.t.sol @@ -2,10 +2,18 @@ pragma solidity ^0.8.18; import "forge-std/Test.sol"; +import "forge-std/Script.sol"; import {LargeContract} from "./LargeContracts.sol"; contract ZkLargeFactoryDependenciesTest is Test { function testLargeFactoryDependenciesAreDeployedInBatches() public { new LargeContract(); } -} \ No newline at end of file +} + +contract ZkLargeFactoryDependenciesScript is Script { + function run() external { + vm.broadcast(); + new LargeContract(); + } +} diff --git a/zk-tests/test.sh b/zk-tests/test.sh index 62659b106..44fa3c601 100755 --- a/zk-tests/test.sh +++ b/zk-tests/test.sh @@ -148,6 +148,9 @@ echo "Running script..." RUST_LOG=warn "${FORGE}" script ./script/Deploy.s.sol:DeployScript --broadcast --private-key "$PRIVATE_KEY" --chain 260 --gas-estimate-multiplier 310 --rpc-url "$RPC_URL" --use "./${SOLC}" --slow -vvv --zk-compile || fail "forge script failed" RUST_LOG=warn "${FORGE}" script ./script/Deploy.s.sol:DeployScript --broadcast --private-key "$PRIVATE_KEY" --chain 260 --gas-estimate-multiplier 310 --rpc-url "$RPC_URL" --use "./${SOLC}" --slow -vvv --zk-compile || fail "forge script failed on 2nd deploy" +echo "Running factory deps script..." +RUST_LOG=warn "${FORGE}" script ./src/LargeFactoryDependencies.t.sol:ZkLargeFactoryDependenciesScript --broadcast --private-key "$PRIVATE_KEY" --chain 260 --gas-estimate-multiplier 310 --rpc-url "$RPC_URL" --use "./${SOLC}" --slow -vvv --zk-startup || fail "forge script failed" + echo "Running NFT script" RUST_LOG=warn "${FORGE}" script ./script/NFT.s.sol:MyScript --broadcast --private-key $PRIVATE_KEY --rpc-url $RPC_URL --use 0.8.20 --zk-startup || fail "forge script failed"