From c0989abbcc40d487d750379f72a50c2d8f1d704a Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 20 May 2022 14:32:29 -0700 Subject: [PATCH 1/2] refactor: Environment.get_network_descriptor returns a ref, not an option zthis eliminates a bunch of unwrap() and expect() calls that can never trigger anyway, because the Environment in question is always an AgentEnvironment anyway. --- src/dfx/src/commands/canister/create.rs | 5 ++--- src/dfx/src/commands/canister/delete.rs | 2 +- src/dfx/src/commands/canister/install.rs | 2 +- src/dfx/src/commands/canister/sign.rs | 1 - src/dfx/src/commands/deploy.rs | 5 ++--- src/dfx/src/commands/diagnose.rs | 3 +-- src/dfx/src/commands/fix.rs | 2 +- src/dfx/src/commands/wallet/mod.rs | 4 ++-- src/dfx/src/commands/wallet/upgrade.rs | 2 +- src/dfx/src/lib/environment.rs | 14 +++++++------- src/dfx/src/lib/models/canister_id_store.rs | 2 +- .../lib/operations/canister/deploy_canisters.rs | 2 +- .../lib/operations/canister/install_canister.rs | 2 +- src/dfx/src/lib/root_key.rs | 12 ++---------- 14 files changed, 23 insertions(+), 35 deletions(-) diff --git a/src/dfx/src/commands/canister/create.rs b/src/dfx/src/commands/canister/create.rs index 7c9d28a8e0..5ab48052af 100644 --- a/src/dfx/src/commands/canister/create.rs +++ b/src/dfx/src/commands/canister/create.rs @@ -75,14 +75,13 @@ pub async fn exec( let with_cycles = opts.with_cycles.as_deref(); let config_interface = config.get_config(); - let network = env.get_network_descriptor().unwrap(); + let network = env.get_network_descriptor(); let proxy_sender; if !opts.no_wallet && !matches!(call_sender, CallSender::Wallet(_)) { let wallet = Identity::get_or_create_wallet_canister( env, - env.get_network_descriptor() - .expect("Couldn't get the network descriptor"), + env.get_network_descriptor(), env.get_selected_identity().expect("No selected identity"), false, ) diff --git a/src/dfx/src/commands/canister/delete.rs b/src/dfx/src/commands/canister/delete.rs index f6e59c2112..cadd5d560e 100644 --- a/src/dfx/src/commands/canister/delete.rs +++ b/src/dfx/src/commands/canister/delete.rs @@ -106,7 +106,7 @@ async fn delete_canister( None => match call_sender { CallSender::Wallet(wallet_id) => Some(*wallet_id), CallSender::SelectedId => { - let network = env.get_network_descriptor().unwrap(); + let network = env.get_network_descriptor(); let agent_env = create_agent_environment(env, Some(network.name.clone()))?; let identity_name = agent_env .get_selected_identity() diff --git a/src/dfx/src/commands/canister/install.rs b/src/dfx/src/commands/canister/install.rs index d30038caba..8fe2a98404 100644 --- a/src/dfx/src/commands/canister/install.rs +++ b/src/dfx/src/commands/canister/install.rs @@ -68,7 +68,7 @@ pub async fn exec( let mode = InstallMode::from_str(opts.mode.as_str()).map_err(|err| anyhow!(err))?; let canister_id_store = CanisterIdStore::for_env(env)?; - let network = env.get_network_descriptor().unwrap(); + let network = env.get_network_descriptor(); if mode == InstallMode::Reinstall && (opts.canister.is_none() || opts.all) { bail!("The --mode=reinstall is only valid when specifying a single canister, because reinstallation destroys all data in the canister."); diff --git a/src/dfx/src/commands/canister/sign.rs b/src/dfx/src/commands/canister/sign.rs index 9be213f004..a47fe39f8f 100644 --- a/src/dfx/src/commands/canister/sign.rs +++ b/src/dfx/src/commands/canister/sign.rs @@ -120,7 +120,6 @@ pub async fn exec( let network = env .get_network_descriptor() - .expect("Cannot get network descriptor from environment.") .providers .first() .expect("Cannot get network provider (url).") diff --git a/src/dfx/src/commands/deploy.rs b/src/dfx/src/commands/deploy.rs index 15bf4eb857..87ca78fa37 100644 --- a/src/dfx/src/commands/deploy.rs +++ b/src/dfx/src/commands/deploy.rs @@ -111,8 +111,7 @@ pub fn exec(env: &dyn Environment, opts: DeployOpts) -> DfxResult { let create_call_sender = if !opts.no_wallet && !matches!(call_sender, CallSender::Wallet(_)) { let wallet = runtime.block_on(Identity::get_or_create_wallet_canister( &env, - env.get_network_descriptor() - .expect("Couldn't get the network descriptor"), + env.get_network_descriptor(), env.get_selected_identity().expect("No selected identity"), false, ))?; @@ -141,7 +140,7 @@ pub fn exec(env: &dyn Environment, opts: DeployOpts) -> DfxResult { fn display_urls(env: &dyn Environment) -> DfxResult { let config = env.get_config_or_anyhow()?; - let network: &NetworkDescriptor = env.get_network_descriptor().unwrap(); + let network: &NetworkDescriptor = env.get_network_descriptor(); let log = env.get_logger(); let canister_id_store = CanisterIdStore::for_env(env)?; diff --git a/src/dfx/src/commands/diagnose.rs b/src/dfx/src/commands/diagnose.rs index 98770122db..65785fa265 100644 --- a/src/dfx/src/commands/diagnose.rs +++ b/src/dfx/src/commands/diagnose.rs @@ -17,7 +17,6 @@ pub struct DiagnoseOpts { pub fn exec(env: &dyn Environment, opts: DiagnoseOpts) -> DfxResult { let env = create_agent_environment(env, opts.network)?; let runtime = Runtime::new().expect("Unable to create a runtime"); - runtime - .block_on(async { migrate(&env, env.get_network_descriptor().unwrap(), false).await })?; + runtime.block_on(async { migrate(&env, env.get_network_descriptor(), false).await })?; Ok(()) } diff --git a/src/dfx/src/commands/fix.rs b/src/dfx/src/commands/fix.rs index 3f10faff9f..5f536ab86b 100644 --- a/src/dfx/src/commands/fix.rs +++ b/src/dfx/src/commands/fix.rs @@ -18,6 +18,6 @@ pub struct FixOpts { pub fn exec(env: &dyn Environment, opts: FixOpts) -> DfxResult { let env = create_agent_environment(env, opts.network)?; let runtime = Runtime::new().expect("Failed to create runtime"); - runtime.block_on(async { migrate(&env, env.get_network_descriptor().unwrap(), true).await })?; + runtime.block_on(async { migrate(&env, env.get_network_descriptor(), true).await })?; Ok(()) } diff --git a/src/dfx/src/commands/wallet/mod.rs b/src/dfx/src/commands/wallet/mod.rs index 6b210f015c..e6c6910f6e 100644 --- a/src/dfx/src/commands/wallet/mod.rs +++ b/src/dfx/src/commands/wallet/mod.rs @@ -91,7 +91,7 @@ where .expect("No selected identity.") .to_string(); // Network descriptor will always be set. - let network = env.get_network_descriptor().unwrap(); + let network = env.get_network_descriptor(); let wallet = Identity::get_or_create_wallet_canister(env, network, &identity_name, false).await?; @@ -128,7 +128,7 @@ async fn get_wallet(env: &dyn Environment) -> DfxResult> { .expect("No selected identity.") .to_string(); // Network descriptor will always be set. - let network = env.get_network_descriptor().unwrap(); + let network = env.get_network_descriptor(); fetch_root_key_if_needed(env).await?; let wallet = Identity::get_or_create_wallet_canister(env, network, &identity_name, false).await?; diff --git a/src/dfx/src/commands/wallet/upgrade.rs b/src/dfx/src/commands/wallet/upgrade.rs index d550e6f78d..586770ac9f 100644 --- a/src/dfx/src/commands/wallet/upgrade.rs +++ b/src/dfx/src/commands/wallet/upgrade.rs @@ -19,7 +19,7 @@ pub async fn exec(env: &dyn Environment, _opts: UpgradeOpts) -> DfxResult { .to_string(); // Network descriptor will always be set. - let network = env.get_network_descriptor().unwrap(); + let network = env.get_network_descriptor(); let canister_id = Identity::wallet_canister_id(env, network, &identity_name)?; diff --git a/src/dfx/src/lib/environment.rs b/src/dfx/src/lib/environment.rs index 3710c18e39..7a7e5cde0d 100644 --- a/src/dfx/src/lib/environment.rs +++ b/src/dfx/src/lib/environment.rs @@ -43,7 +43,7 @@ pub trait Environment { fn get_agent<'a>(&'a self) -> Option<&'a Agent>; #[allow(clippy::needless_lifetimes)] - fn get_network_descriptor<'a>(&'a self) -> Option<&'a NetworkDescriptor>; + fn get_network_descriptor<'a>(&'a self) -> &'a NetworkDescriptor; fn get_logger(&self) -> &slog::Logger; fn new_spinner(&self, message: Cow<'static, str>) -> ProgressBar; @@ -185,10 +185,10 @@ impl Environment for EnvironmentImpl { None } - fn get_network_descriptor(&self) -> Option<&NetworkDescriptor> { - // create an AgentEnvironment explicitly, in order to specify network and agent. - // See install, build for examples. - None + fn get_network_descriptor(&self) -> &NetworkDescriptor { + // It's not valid to call get_network_descriptor on an EnvironmentImpl. + // All of the places that call this have an AgentEnvironment anyway. + panic!("NetworkDescriptor only available from an AgentEnvironment"); } fn get_logger(&self) -> &slog::Logger { @@ -290,8 +290,8 @@ impl<'a> Environment for AgentEnvironment<'a> { Some(&self.agent) } - fn get_network_descriptor(&self) -> Option<&NetworkDescriptor> { - Some(&self.network_descriptor) + fn get_network_descriptor(&self) -> &NetworkDescriptor { + &self.network_descriptor } fn get_logger(&self) -> &slog::Logger { diff --git a/src/dfx/src/lib/models/canister_id_store.rs b/src/dfx/src/lib/models/canister_id_store.rs index 19b4b804c5..e6a175dd12 100644 --- a/src/dfx/src/lib/models/canister_id_store.rs +++ b/src/dfx/src/lib/models/canister_id_store.rs @@ -33,7 +33,7 @@ pub struct CanisterIdStore { impl CanisterIdStore { #[context("Failed to load canister id store.")] pub fn for_env(env: &dyn Environment) -> DfxResult { - let network_descriptor = env.get_network_descriptor().expect("no network descriptor"); + let network_descriptor = env.get_network_descriptor(); let store = CanisterIdStore::for_network(network_descriptor)?; let remote_ids = get_remote_ids(env)?; diff --git a/src/dfx/src/lib/operations/canister/deploy_canisters.rs b/src/dfx/src/lib/operations/canister/deploy_canisters.rs index 3d4ffd6919..8ed39fb52e 100644 --- a/src/dfx/src/lib/operations/canister/deploy_canisters.rs +++ b/src/dfx/src/lib/operations/canister/deploy_canisters.rs @@ -43,7 +43,7 @@ pub async fn deploy_canisters( .ok_or_else(|| anyhow!("Cannot find dfx configuration file in the current working directory. Did you forget to create one?"))?; let initial_canister_id_store = CanisterIdStore::for_env(env)?; - let network = env.get_network_descriptor().unwrap(); + let network = env.get_network_descriptor(); let canisters_to_build = canister_with_dependencies(&config, some_canister)?; diff --git a/src/dfx/src/lib/operations/canister/install_canister.rs b/src/dfx/src/lib/operations/canister/install_canister.rs index 0f98c62c63..b0e99fc30a 100644 --- a/src/dfx/src/lib/operations/canister/install_canister.rs +++ b/src/dfx/src/lib/operations/canister/install_canister.rs @@ -37,7 +37,7 @@ pub async fn install_canister( upgrade_unchanged: bool, ) -> DfxResult { let log = env.get_logger(); - let network = env.get_network_descriptor().unwrap(); + let network = env.get_network_descriptor(); if !network.is_ic && named_canister::get_ui_canister_id(network).is_none() { named_canister::install_ui_canister(env, network, None).await?; } diff --git a/src/dfx/src/lib/root_key.rs b/src/dfx/src/lib/root_key.rs index d4931bbfa1..0f44537ee3 100644 --- a/src/dfx/src/lib/root_key.rs +++ b/src/dfx/src/lib/root_key.rs @@ -10,11 +10,7 @@ pub async fn fetch_root_key_if_needed(env: &dyn Environment) -> DfxResult { .get_agent() .ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?; - if !env - .get_network_descriptor() - .expect("no network descriptor") - .is_ic - { + if !env.get_network_descriptor().is_ic { agent .fetch_root_key() .await @@ -31,11 +27,7 @@ pub async fn fetch_root_key_or_anyhow(env: &dyn Environment) -> DfxResult { .get_agent() .ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?; - if !env - .get_network_descriptor() - .expect("no network descriptor") - .is_ic - { + if !env.get_network_descriptor().is_ic { agent .fetch_root_key() .await From c91392230be01a687980a044ba6d7f85efcf25a5 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 20 May 2022 16:27:09 -0700 Subject: [PATCH 2/2] unreachable instead of panic --- src/dfx/src/lib/environment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dfx/src/lib/environment.rs b/src/dfx/src/lib/environment.rs index 7a7e5cde0d..eb0b979256 100644 --- a/src/dfx/src/lib/environment.rs +++ b/src/dfx/src/lib/environment.rs @@ -188,7 +188,7 @@ impl Environment for EnvironmentImpl { fn get_network_descriptor(&self) -> &NetworkDescriptor { // It's not valid to call get_network_descriptor on an EnvironmentImpl. // All of the places that call this have an AgentEnvironment anyway. - panic!("NetworkDescriptor only available from an AgentEnvironment"); + unreachable!("NetworkDescriptor only available from an AgentEnvironment"); } fn get_logger(&self) -> &slog::Logger {