Skip to content

Commit

Permalink
fix(zkstack): Fix wrong missing private key message (#3334)
Browse files Browse the repository at this point in the history
## What ❔

Add msg_wallet_private_key_not_set and return the correct wallet

## Why ❔

<!-- Why are these changes done? What goal do they contribute to? What
are the principles behind them? -->
<!-- Example: PR templates ensure PR reviewers, observers, and future
iterators are in context about the evolution of repos. -->

## Checklist

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [ ] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [ ] Tests for the changes have been added / updated.
- [ ] Documentation comments have been added / updated.
- [ ] Code has been formatted via `zkstack dev fmt` and `zkstack dev
lint`.
  • Loading branch information
matias-gonz authored Nov 29, 2024
1 parent 38afdd5 commit aaca32b
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 19 deletions.
4 changes: 2 additions & 2 deletions zkstack_cli/crates/zkstack/src/accept_ownership.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use xshell::Shell;

use crate::{
messages::MSG_ACCEPTING_GOVERNANCE_SPINNER,
utils::forge::{check_the_balance, fill_forge_private_key},
utils::forge::{check_the_balance, fill_forge_private_key, WalletOwner},
};

lazy_static! {
Expand Down Expand Up @@ -89,7 +89,7 @@ async fn accept_ownership(
governor: &Wallet,
mut forge: ForgeScript,
) -> anyhow::Result<()> {
forge = fill_forge_private_key(forge, Some(governor))?;
forge = fill_forge_private_key(forge, Some(governor), WalletOwner::Governor)?;
check_the_balance(&forge).await?;
let spinner = Spinner::new(MSG_ACCEPTING_GOVERNANCE_SPINNER);
forge.run(shell)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::{
MSG_CHAIN_NOT_INITIALIZED, MSG_DEPLOYING_L2_CONTRACT_SPINNER,
MSG_L1_SECRETS_MUST_BE_PRESENTED,
},
utils::forge::{check_the_balance, fill_forge_private_key},
utils::forge::{check_the_balance, fill_forge_private_key, WalletOwner},
};

pub enum Deploy2ContractsOption {
Expand Down Expand Up @@ -311,7 +311,11 @@ async fn call_forge(
forge = forge.with_signature(signature);
}

forge = fill_forge_private_key(forge, Some(&ecosystem_config.get_wallets()?.governor))?;
forge = fill_forge_private_key(
forge,
Some(&ecosystem_config.get_wallets()?.governor),
WalletOwner::Governor,
)?;

check_the_balance(&forge).await?;
forge.run(shell)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use xshell::Shell;

use crate::{
messages::{MSG_CHAIN_NOT_INITIALIZED, MSG_L1_SECRETS_MUST_BE_PRESENTED},
utils::forge::{check_the_balance, fill_forge_private_key},
utils::forge::{check_the_balance, fill_forge_private_key, WalletOwner},
};

pub async fn run(args: ForgeScriptArgs, shell: &Shell) -> anyhow::Result<()> {
Expand Down Expand Up @@ -56,7 +56,11 @@ pub async fn deploy_paymaster(
if let Some(address) = sender {
forge = forge.with_sender(address);
} else {
forge = fill_forge_private_key(forge, Some(&chain_config.get_wallets_config()?.governor))?;
forge = fill_forge_private_key(
forge,
Some(&chain_config.get_wallets_config()?.governor),
WalletOwner::Governor,
)?;
}

if broadcast {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
MSG_CHAIN_NOT_INITIALIZED, MSG_CHAIN_REGISTERED, MSG_L1_SECRETS_MUST_BE_PRESENTED,
MSG_REGISTERING_CHAIN_SPINNER,
},
utils::forge::{check_the_balance, fill_forge_private_key},
utils::forge::{check_the_balance, fill_forge_private_key, WalletOwner},
};

pub async fn run(args: ForgeScriptArgs, shell: &Shell) -> anyhow::Result<()> {
Expand Down Expand Up @@ -81,7 +81,11 @@ pub async fn register_chain(
if let Some(address) = sender {
forge = forge.with_sender(address);
} else {
forge = fill_forge_private_key(forge, Some(&config.get_wallets()?.governor))?;
forge = fill_forge_private_key(
forge,
Some(&config.get_wallets()?.governor),
WalletOwner::Governor,
)?;
check_the_balance(&forge).await?;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::{
MSG_TOKEN_MULTIPLIER_SETTER_UPDATED_TO, MSG_UPDATING_TOKEN_MULTIPLIER_SETTER_SPINNER,
MSG_WALLETS_CONFIG_MUST_BE_PRESENT, MSG_WALLET_TOKEN_MULTIPLIER_SETTER_NOT_FOUND,
},
utils::forge::{check_the_balance, fill_forge_private_key},
utils::forge::{check_the_balance, fill_forge_private_key, WalletOwner},
};

lazy_static! {
Expand Down Expand Up @@ -109,7 +109,7 @@ async fn update_token_multiplier_setter(
governor: &Wallet,
mut forge: ForgeScript,
) -> anyhow::Result<()> {
forge = fill_forge_private_key(forge, Some(governor))?;
forge = fill_forge_private_key(forge, Some(governor), WalletOwner::Governor)?;
check_the_balance(&forge).await?;
forge.run(shell)?;
Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use xshell::Shell;

use crate::{
messages::{MSG_DEPLOYING_PAYMASTER, MSG_L1_SECRETS_MUST_BE_PRESENTED},
utils::forge::{check_the_balance, fill_forge_private_key},
utils::forge::{check_the_balance, fill_forge_private_key, WalletOwner},
};

pub async fn setup_legacy_bridge(
Expand Down Expand Up @@ -59,7 +59,11 @@ pub async fn setup_legacy_bridge(
)
.with_broadcast();

forge = fill_forge_private_key(forge, Some(&ecosystem_config.get_wallets()?.governor))?;
forge = fill_forge_private_key(
forge,
Some(&ecosystem_config.get_wallets()?.governor),
WalletOwner::Governor,
)?;

let spinner = Spinner::new(MSG_DEPLOYING_PAYMASTER);
check_the_balance(&forge).await?;
Expand Down
8 changes: 6 additions & 2 deletions zkstack_cli/crates/zkstack/src/commands/ecosystem/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use config::{
use types::{L1Network, ProverMode};
use xshell::Shell;

use crate::utils::forge::{check_the_balance, fill_forge_private_key};
use crate::utils::forge::{check_the_balance, fill_forge_private_key, WalletOwner};

pub async fn deploy_l1(
shell: &Shell,
Expand Down Expand Up @@ -54,7 +54,11 @@ pub async fn deploy_l1(
if let Some(address) = sender {
forge = forge.with_sender(address);
} else {
forge = fill_forge_private_key(forge, wallets_config.deployer.as_ref())?;
forge = fill_forge_private_key(
forge,
wallets_config.deployer.as_ref(),
WalletOwner::Deployer,
)?;
}

if broadcast {
Expand Down
8 changes: 6 additions & 2 deletions zkstack_cli/crates/zkstack/src/commands/ecosystem/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use crate::{
MSG_ECOSYSTEM_CONTRACTS_PATH_PROMPT, MSG_INITIALIZING_ECOSYSTEM,
MSG_INTALLING_DEPS_SPINNER,
},
utils::forge::{check_the_balance, fill_forge_private_key},
utils::forge::{check_the_balance, fill_forge_private_key, WalletOwner},
};

pub async fn run(args: EcosystemInitArgs, shell: &Shell) -> anyhow::Result<()> {
Expand Down Expand Up @@ -150,7 +150,11 @@ async fn deploy_erc20(
.with_rpc_url(l1_rpc_url)
.with_broadcast();

forge = fill_forge_private_key(forge, ecosystem_config.get_wallets()?.deployer.as_ref())?;
forge = fill_forge_private_key(
forge,
ecosystem_config.get_wallets()?.deployer.as_ref(),
WalletOwner::Deployer,
)?;

let spinner = Spinner::new(MSG_DEPLOYING_ERC20_SPINNER);
check_the_balance(&forge).await?;
Expand Down
12 changes: 11 additions & 1 deletion zkstack_cli/crates/zkstack/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use ethers::{
use url::Url;
use zksync_consensus_roles::attester;

use crate::utils::forge::WalletOwner;

pub(super) const MSG_SETUP_KEYS_DOWNLOAD_SELECTION_PROMPT: &str =
"Do you want to download the setup keys or generate them?";
pub(super) const MSG_SETUP_KEYS_REGION_PROMPT: &str =
Expand Down Expand Up @@ -334,7 +336,15 @@ pub(super) fn msg_explorer_chain_not_initialized(chain: &str) -> String {
}

/// Forge utils related messages
pub(super) const MSG_DEPLOYER_PK_NOT_SET_ERR: &str = "Deployer private key is not set";
pub(super) fn msg_wallet_private_key_not_set(wallet_owner: WalletOwner) -> String {
format!(
"{} private key is not set",
match wallet_owner {
WalletOwner::Governor => "Governor",
WalletOwner::Deployer => "Deployer",
}
)
}

pub(super) fn msg_address_doesnt_have_enough_money_prompt(
address: &H160,
Expand Down
10 changes: 8 additions & 2 deletions zkstack_cli/crates/zkstack/src/utils/forge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,24 @@ use ethers::types::U256;

use crate::{
consts::MINIMUM_BALANCE_FOR_WALLET,
messages::{msg_address_doesnt_have_enough_money_prompt, MSG_DEPLOYER_PK_NOT_SET_ERR},
messages::{msg_address_doesnt_have_enough_money_prompt, msg_wallet_private_key_not_set},
};

pub enum WalletOwner {
Governor,
Deployer,
}

pub fn fill_forge_private_key(
mut forge: ForgeScript,
wallet: Option<&Wallet>,
wallet_owner: WalletOwner,
) -> anyhow::Result<ForgeScript> {
if !forge.wallet_args_passed() {
forge = forge.with_private_key(
wallet
.and_then(|w| w.private_key_h256())
.context(MSG_DEPLOYER_PK_NOT_SET_ERR)?,
.context(msg_wallet_private_key_not_set(wallet_owner))?,
);
}
Ok(forge)
Expand Down

0 comments on commit aaca32b

Please sign in to comment.