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: Remove wrong estimation in create for zksync transactions #864

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 14 additions & 21 deletions crates/forge/bin/cmd/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ pub struct CreateArgs {

#[command(flatten)]
retry: RetryArgs,

/// Gas per pubdata
#[clap(long = "zk-gas-per-pubdata", value_name = "GAS_PER_PUBDATA")]
pub zk_gas_per_pubdata: Option<u64>,
}

#[derive(Debug, Default)]
Expand All @@ -128,7 +132,7 @@ impl CreateArgs {
/// Executes the command to create a contract
pub async fn run(mut self) -> Result<()> {
let mut config = self.try_load_config_emit_warnings()?;

let timeout = config.transaction_timeout;
// Install missing dependencies.
if install::install_missing_dependencies(&mut config) && config.auto_detect_remappings {
// need to re-configure here to also catch additional remappings
Expand Down Expand Up @@ -301,7 +305,7 @@ impl CreateArgs {
provider,
chain_id,
deployer,
config.transaction_timeout,
timeout,
id,
zk_data,
)
Expand Down Expand Up @@ -659,7 +663,6 @@ impl CreateArgs {
e
}
})?;
let is_legacy = self.tx.legacy || Chain::from(chain).is_legacy();

deployer.tx = deployer.tx.with_factory_deps(
zk_data.factory_deps.clone().into_iter().map(|dep| dep.into()).collect(),
Expand Down Expand Up @@ -692,24 +695,13 @@ impl CreateArgs {
deployer.tx.set_gas_price(gas_price);

// estimate fee
foundry_zksync_core::estimate_fee(&mut deployer.tx, &provider, 130, None).await?;

if !is_legacy {
let estimate = provider.estimate_eip1559_fees(None).await.wrap_err("Failed to estimate EIP1559 fees. This chain might not support EIP1559, try adding --legacy to your command.")?;
let priority_fee = if let Some(priority_fee) = self.tx.priority_gas_price {
priority_fee.to()
} else {
estimate.max_priority_fee_per_gas
};
let max_fee = if let Some(max_fee) = self.tx.gas_price {
max_fee.to()
} else {
estimate.max_fee_per_gas
};

deployer.tx.set_max_fee_per_gas(max_fee);
deployer.tx.set_max_priority_fee_per_gas(priority_fee);
}
foundry_zksync_core::estimate_fee(
&mut deployer.tx,
&provider,
130,
self.zk_gas_per_pubdata,
)
.await?;

if let Some(gas_limit) = self.tx.gas_limit {
deployer.tx.set_gas_limit(gas_limit.to::<u64>());
Expand Down Expand Up @@ -989,6 +981,7 @@ where
.send_transaction(self.tx)
.await?
.with_required_confirmations(self.confs as u64)
.with_timeout(Some(std::time::Duration::from_secs(self.timeout)))
.get_receipt()
.await?;

Expand Down
5 changes: 5 additions & 0 deletions crates/forge/tests/it/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,7 @@ pub fn deploy_zk_contract(
url: &str,
private_key: &str,
contract_path: &str,
extra_args: Option<&[&str]>,
) -> Result<String, String> {
cmd.forge_fuse().args([
"create",
Expand All @@ -635,6 +636,10 @@ pub fn deploy_zk_contract(
private_key,
]);

if let Some(args) = extra_args {
cmd.args(args);
}

let output = cmd.assert_success();
let output = output.get_output();
let stdout = output.stdout_lossy();
Expand Down
56 changes: 54 additions & 2 deletions crates/forge/tests/it/zk/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ forgetest_async!(forge_zk_can_deploy_erc20, |prj, cmd| {
ZkSyncNode::rich_wallets().next().map(|(_, pk, _)| pk).expect("No rich wallets available");

let erc20_address =
deploy_zk_contract(&mut cmd, url.as_str(), private_key, "./src/ERC20.sol:MyToken")
deploy_zk_contract(&mut cmd, url.as_str(), private_key, "./src/ERC20.sol:MyToken", None)
.expect("Failed to deploy ERC20 contract");

assert!(!erc20_address.is_empty(), "Deployed address should not be empty");
Expand All @@ -43,10 +43,11 @@ forgetest_async!(forge_zk_can_deploy_contracts_and_cast_a_transaction, |prj, cmd
url.as_str(),
private_key,
"./src/TokenReceiver.sol:TokenReceiver",
None,
)
.expect("Failed to deploy TokenReceiver contract");
let erc_20_address =
deploy_zk_contract(&mut cmd, url.as_str(), private_key, "./src/ERC20.sol:MyToken")
deploy_zk_contract(&mut cmd, url.as_str(), private_key, "./src/ERC20.sol:MyToken", None)
.expect("Failed to deploy ERC20 contract");

cmd.cast_fuse().args([
Expand All @@ -66,3 +67,54 @@ forgetest_async!(forge_zk_can_deploy_contracts_and_cast_a_transaction, |prj, cmd
assert!(stdout.contains("transactionHash"), "Transaction hash not found in output");
assert!(stdout.contains("success"), "Transaction was not successful");
});

forgetest_async!(forge_zk_can_deploy_contracts_with_gas_per_pubdata_and_succeed, |prj, cmd| {
util::initialize(prj.root());
prj.add_source("ERC20.sol", include_str!("../../../../../testdata/zk/ERC20.sol")).unwrap();

let node = ZkSyncNode::start().await;
let url = node.url();

let private_key =
ZkSyncNode::rich_wallets().next().map(|(_, pk, _)| pk).expect("No rich wallets available");

deploy_zk_contract(
&mut cmd,
url.as_str(),
private_key,
"./src/ERC20.sol:MyToken",
Some(&["--zk-gas-per-pubdata", "3000"]),
)
.expect("Failed to deploy ERC20 contract");
});

forgetest_async!(
forge_zk_can_deploy_contracts_with_invalid_gas_per_pubdata_and_fail,
|prj, cmd| {
util::initialize(prj.root());
prj.add_source("ERC20.sol", include_str!("../../../../../testdata/zk/ERC20.sol")).unwrap();

let node = ZkSyncNode::start().await;
let url = node.url();

let private_key = ZkSyncNode::rich_wallets()
.next()
.map(|(_, pk, _)| pk)
.expect("No rich wallets available");
cmd.forge_fuse().args([
"create",
"--zk-startup",
"./src/ERC20.sol:MyToken",
"--rpc-url",
url.as_str(),
"--private-key",
private_key,
"--timeout",
"1",
"--zk-gas-per-pubdata",
"1",
]);

cmd.assert_failure();
}
);
1 change: 1 addition & 0 deletions crates/forge/tests/it/zk/linking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ forgetest_async!(
"127.0.0.1:1234",
"0x0000000000000000000000000000000000000000000000000000000000000000",
"./src/WithLibraries.sol:UsesFoo",
None,
)
.expect("Failed to deploy UsesFoo contract");

Expand Down
Loading