Skip to content

Commit cf0a88d

Browse files
Jrigadadutterbutternbaztec
authored
fix: Remove wrong estimation in create for zksync transactions (#864)
* Remove wrong estimation in create for zksync transactions, add timeout parameter being retrieve wrong and gas per pubdata parameter * Fix estimation on non legacy chains for scripts * Format --------- Co-authored-by: Dustin Brickwood <[email protected]> Co-authored-by: Nisheeth Barthwal <[email protected]>
1 parent 62af6f9 commit cf0a88d

File tree

5 files changed

+92
-31
lines changed

5 files changed

+92
-31
lines changed

‎crates/forge/bin/cmd/create.rs

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ pub struct CreateArgs {
113113

114114
#[command(flatten)]
115115
retry: RetryArgs,
116+
117+
/// Gas per pubdata
118+
#[clap(long = "zk-gas-per-pubdata", value_name = "GAS_PER_PUBDATA")]
119+
pub zk_gas_per_pubdata: Option<u64>,
116120
}
117121

118122
#[derive(Debug, Default)]
@@ -128,7 +132,7 @@ impl CreateArgs {
128132
/// Executes the command to create a contract
129133
pub async fn run(mut self) -> Result<()> {
130134
let mut config = self.try_load_config_emit_warnings()?;
131-
135+
let timeout = config.transaction_timeout;
132136
// Install missing dependencies.
133137
if install::install_missing_dependencies(&mut config) && config.auto_detect_remappings {
134138
// need to re-configure here to also catch additional remappings
@@ -301,7 +305,7 @@ impl CreateArgs {
301305
provider,
302306
chain_id,
303307
deployer,
304-
config.transaction_timeout,
308+
timeout,
305309
id,
306310
zk_data,
307311
)
@@ -659,7 +663,6 @@ impl CreateArgs {
659663
e
660664
}
661665
})?;
662-
let is_legacy = self.tx.legacy || Chain::from(chain).is_legacy();
663666

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

694697
// estimate fee
695-
foundry_zksync_core::estimate_fee(&mut deployer.tx, &provider, 130, None).await?;
696-
697-
if !is_legacy {
698-
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.")?;
699-
let priority_fee = if let Some(priority_fee) = self.tx.priority_gas_price {
700-
priority_fee.to()
701-
} else {
702-
estimate.max_priority_fee_per_gas
703-
};
704-
let max_fee = if let Some(max_fee) = self.tx.gas_price {
705-
max_fee.to()
706-
} else {
707-
estimate.max_fee_per_gas
708-
};
709-
710-
deployer.tx.set_max_fee_per_gas(max_fee);
711-
deployer.tx.set_max_priority_fee_per_gas(priority_fee);
712-
}
698+
foundry_zksync_core::estimate_fee(
699+
&mut deployer.tx,
700+
&provider,
701+
130,
702+
self.zk_gas_per_pubdata,
703+
)
704+
.await?;
713705

714706
if let Some(gas_limit) = self.tx.gas_limit {
715707
deployer.tx.set_gas_limit(gas_limit.to::<u64>());
@@ -989,6 +981,7 @@ where
989981
.send_transaction(self.tx)
990982
.await?
991983
.with_required_confirmations(self.confs as u64)
984+
.with_timeout(Some(std::time::Duration::from_secs(self.timeout)))
992985
.get_receipt()
993986
.await?;
994987

‎crates/forge/tests/it/test_helpers.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,7 @@ pub fn deploy_zk_contract(
624624
url: &str,
625625
private_key: &str,
626626
contract_path: &str,
627+
extra_args: Option<&[&str]>,
627628
) -> Result<String, String> {
628629
cmd.forge_fuse().args([
629630
"create",
@@ -635,6 +636,10 @@ pub fn deploy_zk_contract(
635636
private_key,
636637
]);
637638

639+
if let Some(args) = extra_args {
640+
cmd.args(args);
641+
}
642+
638643
let output = cmd.assert_success();
639644
let output = output.get_output();
640645
let stdout = output.stdout_lossy();

‎crates/forge/tests/it/zk/create.rs

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ forgetest_async!(forge_zk_can_deploy_erc20, |prj, cmd| {
1717
ZkSyncNode::rich_wallets().next().map(|(_, pk, _)| pk).expect("No rich wallets available");
1818

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

2323
assert!(!erc20_address.is_empty(), "Deployed address should not be empty");
@@ -43,10 +43,11 @@ forgetest_async!(forge_zk_can_deploy_contracts_and_cast_a_transaction, |prj, cmd
4343
url.as_str(),
4444
private_key,
4545
"./src/TokenReceiver.sol:TokenReceiver",
46+
None,
4647
)
4748
.expect("Failed to deploy TokenReceiver contract");
4849
let erc_20_address =
49-
deploy_zk_contract(&mut cmd, url.as_str(), private_key, "./src/ERC20.sol:MyToken")
50+
deploy_zk_contract(&mut cmd, url.as_str(), private_key, "./src/ERC20.sol:MyToken", None)
5051
.expect("Failed to deploy ERC20 contract");
5152

5253
cmd.cast_fuse().args([
@@ -66,3 +67,54 @@ forgetest_async!(forge_zk_can_deploy_contracts_and_cast_a_transaction, |prj, cmd
6667
assert!(stdout.contains("transactionHash"), "Transaction hash not found in output");
6768
assert!(stdout.contains("success"), "Transaction was not successful");
6869
});
70+
71+
forgetest_async!(forge_zk_can_deploy_contracts_with_gas_per_pubdata_and_succeed, |prj, cmd| {
72+
util::initialize(prj.root());
73+
prj.add_source("ERC20.sol", include_str!("../../../../../testdata/zk/ERC20.sol")).unwrap();
74+
75+
let node = ZkSyncNode::start().await;
76+
let url = node.url();
77+
78+
let private_key =
79+
ZkSyncNode::rich_wallets().next().map(|(_, pk, _)| pk).expect("No rich wallets available");
80+
81+
deploy_zk_contract(
82+
&mut cmd,
83+
url.as_str(),
84+
private_key,
85+
"./src/ERC20.sol:MyToken",
86+
Some(&["--zk-gas-per-pubdata", "3000"]),
87+
)
88+
.expect("Failed to deploy ERC20 contract");
89+
});
90+
91+
forgetest_async!(
92+
forge_zk_can_deploy_contracts_with_invalid_gas_per_pubdata_and_fail,
93+
|prj, cmd| {
94+
util::initialize(prj.root());
95+
prj.add_source("ERC20.sol", include_str!("../../../../../testdata/zk/ERC20.sol")).unwrap();
96+
97+
let node = ZkSyncNode::start().await;
98+
let url = node.url();
99+
100+
let private_key = ZkSyncNode::rich_wallets()
101+
.next()
102+
.map(|(_, pk, _)| pk)
103+
.expect("No rich wallets available");
104+
cmd.forge_fuse().args([
105+
"create",
106+
"--zk-startup",
107+
"./src/ERC20.sol:MyToken",
108+
"--rpc-url",
109+
url.as_str(),
110+
"--private-key",
111+
private_key,
112+
"--timeout",
113+
"1",
114+
"--zk-gas-per-pubdata",
115+
"1",
116+
]);
117+
118+
cmd.assert_failure();
119+
}
120+
);

‎crates/forge/tests/it/zk/linking.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ forgetest_async!(
3737
"127.0.0.1:1234",
3838
"0x0000000000000000000000000000000000000000000000000000000000000000",
3939
"./src/WithLibraries.sol:UsesFoo",
40+
None,
4041
)
4142
.expect("Failed to deploy UsesFoo contract");
4243

‎crates/script/src/broadcast.rs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -348,17 +348,27 @@ impl BundledState {
348348
}),
349349
),
350350
(false, _, _) => {
351-
let mut fees = 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.")?;
351+
if self.script_config.config.zksync.run_in_zk_mode() {
352+
// NOTE(zk): We need to avoid estimating eip1559 fees for zk
353+
// transactions as the fee is estimated
354+
// later. This branch is for non legacy chains (zkchains).
355+
(Some(provider.get_gas_price().await?), None)
356+
} else {
357+
let mut fees = provider
358+
.estimate_eip1559_fees(None)
359+
.await
360+
.wrap_err("Failed to estimate EIP1559 fees. This chain might not support EIP1559, try adding --legacy to your command.")?;
352361

353-
if let Some(gas_price) = self.args.with_gas_price {
354-
fees.max_fee_per_gas = gas_price.to();
355-
}
362+
if let Some(gas_price) = self.args.with_gas_price {
363+
fees.max_fee_per_gas = gas_price.to();
364+
}
356365

357-
if let Some(priority_gas_price) = self.args.priority_gas_price {
358-
fees.max_priority_fee_per_gas = priority_gas_price.to();
359-
}
366+
if let Some(priority_gas_price) = self.args.priority_gas_price {
367+
fees.max_priority_fee_per_gas = priority_gas_price.to();
368+
}
360369

361-
(None, Some(fees))
370+
(None, Some(fees))
371+
}
362372
}
363373
};
364374

0 commit comments

Comments
 (0)