Skip to content

Commit

Permalink
refactor: Environment.get_agent() returns an agent or panics (#3408)
Browse files Browse the repository at this point in the history
The Environment on the call stack is either an AgentEnvironment, or it isn't.  All of these `.ok_or_else`/`.expect` checks either always fail, or never fail.  They never fail.  They're all unreachable.

This PR changes `Environment::get_agent()` to return an `&Agent` rather than an `Option<&Agent>`, and removes all of the unreachable error checking.
  • Loading branch information
ericswanson-dfinity authored Oct 4, 2023
1 parent 9d1232d commit d8cc522
Show file tree
Hide file tree
Showing 29 changed files with 52 additions and 115 deletions.
4 changes: 1 addition & 3 deletions src/dfx/src/commands/canister/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,7 @@ pub async fn exec(
// Get the argument, get the type, convert the argument to the type and return
// an error if any of it doesn't work.
let arg_value = blob_from_arguments(arguments, opts.random.as_deref(), arg_type, &method_type)?;
let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

fetch_root_key_if_needed(env).await?;

Expand Down
6 changes: 2 additions & 4 deletions src/dfx/src/commands/canister/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::lib::operations::canister::{
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::assets::wallet_wasm;
use crate::util::blob_from_arguments;
use anyhow::{anyhow, Context};
use anyhow::Context;
use candid::Principal;
use clap::Parser;
use dfx_core::canister::build_wallet_canister;
Expand Down Expand Up @@ -149,9 +149,7 @@ async fn delete_canister(
"Canister {canister} has not been stopped. Delete anyway?"
))?;
}
let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();
let mgr = ManagementCanister::create(agent);
let canister_id =
Principal::from_text(canister).or_else(|_| canister_id_store.get(canister))?;
Expand Down
4 changes: 1 addition & 3 deletions src/dfx/src/commands/canister/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ pub struct InfoOpts {
}

pub async fn exec(env: &dyn Environment, opts: InfoOpts) -> DfxResult {
let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

let callee_canister = opts.canister.as_str();
let canister_id_store = env.get_canister_id_store()?;
Expand Down
6 changes: 2 additions & 4 deletions src/dfx/src/commands/canister/metadata.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::lib::error::DfxResult;
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::Environment;
use anyhow::{anyhow, Context};
use anyhow::Context;
use candid::Principal;
use clap::Parser;
use std::io::{stdout, Write};
Expand All @@ -17,9 +17,7 @@ pub struct CanisterMetadataOpts {
}

pub async fn exec(env: &dyn Environment, opts: CanisterMetadataOpts) -> DfxResult {
let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

let callee_canister = opts.canister_name.as_str();
let canister_id_store = env.get_canister_id_store()?;
Expand Down
6 changes: 2 additions & 4 deletions src/dfx/src/commands/canister/request_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::lib::error::{DfxError, DfxResult};
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::clap::parsers;
use crate::util::print_idl_blob;
use anyhow::{anyhow, Context};
use anyhow::Context;
use backoff::backoff::Backoff;
use backoff::ExponentialBackoff;
use candid::Principal;
Expand Down Expand Up @@ -36,9 +36,7 @@ pub struct RequestStatusOpts {
pub async fn exec(env: &dyn Environment, opts: RequestStatusOpts) -> DfxResult {
let request_id =
RequestId::from_str(&opts.request_id[2..]).context("Invalid argument: request_id")?;
let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

fetch_root_key_if_needed(env).await?;

Expand Down
4 changes: 1 addition & 3 deletions src/dfx/src/commands/canister/sign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,7 @@ pub async fn exec(
// Get the argument, get the type, convert the argument to the type and return
// an error if any of it doesn't work.
let arg_value = blob_from_arguments(arguments, opts.random.as_deref(), arg_type, &method_type)?;
let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

let network = env
.get_network_descriptor()
Expand Down
6 changes: 2 additions & 4 deletions src/dfx/src/commands/deps/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::lib::deps::{
use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::lib::root_key::fetch_root_key_if_needed;
use anyhow::anyhow;

use candid::Principal;
use clap::Parser;
use fn_error_context::context;
Expand Down Expand Up @@ -38,9 +38,7 @@ pub async fn exec(env: &dyn Environment, opts: DepsDeployOpts) -> DfxResult {
let init_json = load_init_json(&project_root)?;

fetch_root_key_if_needed(env).await?;
let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

let canister_ids = match &opts.canister {
Some(canister) => {
Expand Down
4 changes: 1 addition & 3 deletions src/dfx/src/commands/deps/pull.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ pub async fn exec(env: &dyn Environment, opts: DepsPullOpts) -> DfxResult {

fetch_root_key_if_needed(&env).await?;

let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

let all_dependencies =
resolve_all_dependencies(agent, logger, &pull_canisters_in_config).await?;
Expand Down
4 changes: 1 addition & 3 deletions src/dfx/src/commands/identity/set_wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ pub fn exec(env: &dyn Environment, opts: SetWalletOpts, network: NetworkOpt) ->
"Skipping verification of availability of the canister on the network due to --force..."
);
} else {
let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

runtime
.block_on(async {
Expand Down
4 changes: 1 addition & 3 deletions src/dfx/src/commands/ledger/balance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ pub async fn exec(env: &dyn Environment, opts: BalanceOpts) -> DfxResult {
|v| AccountIdentifier::from_str(&v),
)
.map_err(|err| anyhow!(err))?;
let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

let balance = ledger::balance(agent, &acc_id, opts.ledger_canister_id).await?;

Expand Down
6 changes: 2 additions & 4 deletions src/dfx/src/commands/ledger/create_canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::lib::nns_types::icpts::{ICPTs, TRANSACTION_FEE};
use crate::lib::operations::cmc::{notify_create, transfer_cmc};
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::clap::parsers::e8s_parser;
use anyhow::{anyhow, bail, Context};
use anyhow::{bail, Context};
use candid::Principal;
use clap::Parser;

Expand Down Expand Up @@ -71,9 +71,7 @@ pub async fn exec(env: &dyn Environment, opts: CreateCanisterOpts) -> DfxResult
)
})?;

let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

fetch_root_key_if_needed(env).await?;

Expand Down
6 changes: 2 additions & 4 deletions src/dfx/src/commands/ledger/notify/create_canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::lib::ledger_types::NotifyError::Refunded;
use crate::lib::operations::cmc::notify_create;
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::lib::{environment::Environment, error::DfxResult};
use anyhow::{anyhow, bail};
use anyhow::bail;
use candid::Principal;
use clap::Parser;

Expand All @@ -27,9 +27,7 @@ pub async fn exec(env: &dyn Environment, opts: NotifyCreateOpts) -> DfxResult {
let block_height = opts.block_height;
let controller = opts.controller;

let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

fetch_root_key_if_needed(env).await?;

Expand Down
6 changes: 2 additions & 4 deletions src/dfx/src/commands/ledger/notify/top_up.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::lib::ledger_types::NotifyError::Refunded;
use crate::lib::operations::cmc::notify_top_up;
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::lib::{environment::Environment, error::DfxResult};
use anyhow::{anyhow, bail};
use anyhow::bail;
use candid::Principal;
use clap::Parser;

Expand All @@ -21,9 +21,7 @@ pub async fn exec(env: &dyn Environment, opts: NotifyTopUpOpts) -> DfxResult {
let block_height = opts.block_height;
let canister = opts.canister;

let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

fetch_root_key_if_needed(env).await?;

Expand Down
6 changes: 2 additions & 4 deletions src/dfx/src/commands/ledger/show_subnet_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::lib::environment::Environment;
use crate::lib::error::DfxResult;
use crate::lib::ledger_types::{GetSubnetTypesToSubnetsResult, MAINNET_CYCLE_MINTER_CANISTER_ID};
use crate::lib::root_key::fetch_root_key_if_needed;
use anyhow::{anyhow, Context};
use anyhow::Context;
use candid::{Decode, Encode, Principal};
use clap::Parser;

Expand All @@ -17,9 +17,7 @@ pub struct ShowSubnetTypesOpts {
}

pub async fn exec(env: &dyn Environment, opts: ShowSubnetTypesOpts) -> DfxResult {
let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

fetch_root_key_if_needed(env).await?;

Expand Down
6 changes: 2 additions & 4 deletions src/dfx/src/commands/ledger/top_up.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::lib::nns_types::icpts::{ICPTs, TRANSACTION_FEE};
use crate::lib::operations::cmc::{notify_top_up, transfer_cmc};
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::clap::parsers::e8s_parser;
use anyhow::{anyhow, bail, Context};
use anyhow::{bail, Context};
use candid::Principal;
use clap::Parser;

Expand Down Expand Up @@ -65,9 +65,7 @@ pub async fn exec(env: &dyn Environment, opts: TopUpOpts) -> DfxResult {
)
})?;

let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

fetch_root_key_if_needed(env).await?;

Expand Down
4 changes: 1 addition & 3 deletions src/dfx/src/commands/ledger/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,7 @@ pub async fn exec(env: &dyn Environment, opts: TransferOpts) -> DfxResult {
})?
.to_address();

let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

fetch_root_key_if_needed(env).await?;

Expand Down
2 changes: 1 addition & 1 deletion src/dfx/src/commands/quickstart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub struct QuickstartOpts;

pub fn exec(env: &dyn Environment, _: QuickstartOpts) -> DfxResult {
let env = create_agent_environment(env, Some("ic".to_string()))?;
let agent = env.get_agent().expect("Unable to create agent");
let agent = env.get_agent();
let ident = env.get_selected_identity().unwrap();
let principal = env.get_selected_identity_principal().unwrap();
eprintln!("Your DFX user principal: {principal}");
Expand Down
4 changes: 1 addition & 3 deletions src/dfx/src/commands/wallet/redeem_faucet_coupon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ pub async fn exec(env: &dyn Environment, opts: RedeemFaucetCouponOpts) -> DfxRes
} else {
Principal::from_text(DEFAULT_FAUCET_PRINCIPAL).unwrap()
};
let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();
if fetch_root_key_if_needed(env).await.is_err() {
bail!("Failed to connect to the local replica. Did you forget to use '--network ic'?");
} else if !env.get_network_descriptor().is_ic {
Expand Down
10 changes: 3 additions & 7 deletions src/dfx/src/commands/wallet/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::lib::identity::wallet::wallet_canister_id;
use crate::lib::operations::canister::install_canister::install_wallet;
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::lib::state_tree::canister_info::read_state_tree_canister_module_hash;
use anyhow::{anyhow, bail};
use anyhow::bail;
use clap::Parser;
use ic_utils::interfaces::management_canister::builders::InstallMode;

Expand All @@ -31,9 +31,7 @@ pub async fn exec(env: &dyn Environment, _opts: UpgradeOpts) -> DfxResult {
);
};

let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

fetch_root_key_if_needed(env).await?;
if read_state_tree_canister_module_hash(agent, canister_id)
Expand All @@ -43,9 +41,7 @@ pub async fn exec(env: &dyn Environment, _opts: UpgradeOpts) -> DfxResult {
bail!("The cycles wallet canister is empty. Try running `dfx identity deploy-wallet` to install code for the cycles wallet in this canister.")
}

let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();

install_wallet(env, agent, canister_id, InstallMode::Upgrade).await?;

Expand Down
12 changes: 5 additions & 7 deletions src/dfx/src/lib/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub trait Environment {

// Explicit lifetimes are actually needed for mockall to work properly.
#[allow(clippy::needless_lifetimes)]
fn get_agent<'a>(&'a self) -> Option<&'a Agent>;
fn get_agent<'a>(&'a self) -> &'a Agent;

#[allow(clippy::needless_lifetimes)]
fn get_network_descriptor<'a>(&'a self) -> &'a NetworkDescriptor;
Expand Down Expand Up @@ -210,10 +210,8 @@ impl Environment for EnvironmentImpl {
&self.identity_override
}

fn get_agent(&self) -> Option<&Agent> {
// create an AgentEnvironment explicitly, in order to specify network and agent.
// See install, build for examples.
None
fn get_agent(&self) -> &Agent {
unreachable!("Agent only available from an AgentEnvironment");
}

fn get_network_descriptor(&self) -> &NetworkDescriptor {
Expand Down Expand Up @@ -337,8 +335,8 @@ impl<'a> Environment for AgentEnvironment<'a> {
self.backend.get_identity_override()
}

fn get_agent(&self) -> Option<&Agent> {
Some(&self.agent)
fn get_agent(&self) -> &Agent {
&self.agent
}

fn get_network_descriptor(&self) -> &NetworkDescriptor {
Expand Down
10 changes: 3 additions & 7 deletions src/dfx/src/lib/identity/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::lib::error::DfxResult;
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::assets::wallet_wasm;
use crate::Environment;
use anyhow::{anyhow, bail, Context};
use anyhow::{bail, Context};
use candid::Principal;
use dfx_core::canister::build_wallet_canister;
use dfx_core::config::directories::get_user_dfx_config_dir;
Expand Down Expand Up @@ -83,9 +83,7 @@ pub async fn create_wallet(
some_canister_id: Option<Principal>,
) -> DfxResult<Principal> {
fetch_root_key_if_needed(env).await?;
let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();
let mgr = ManagementCanister::create(agent);
info!(
env.get_logger(),
Expand Down Expand Up @@ -161,9 +159,7 @@ pub async fn get_or_create_wallet_canister<'env>(
// without this async block, #[context] gives a spurious error
async {
let wallet_canister_id = get_or_create_wallet(env, network, name).await?;
let agent = env
.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?;
let agent = env.get_agent();
build_wallet_canister(wallet_canister_id, agent)
.await
.map_err(Into::into)
Expand Down
4 changes: 1 addition & 3 deletions src/dfx/src/lib/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ pub async fn migrate(env: &dyn Environment, network: &NetworkDescriptor, fix: bo
fetch_root_key_if_needed(env).await?;
let config = env.get_config_or_anyhow()?;
let config = config.get_config();
let agent = env
.get_agent()
.expect("Could not get agent from environment");
let agent = env.get_agent();
let mut mgr = env.new_identity_manager()?;
let ident = mgr.instantiate_selected_identity(env.get_logger())?;
let mut did_migrate = false;
Expand Down
5 changes: 1 addition & 4 deletions src/dfx/src/lib/named_canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,7 @@ pub async fn install_ui_canister(
));
}
fetch_root_key_if_needed(env).await?;
let mgr = ManagementCanister::create(
env.get_agent()
.ok_or_else(|| anyhow!("Cannot get HTTP client from environment."))?,
);
let mgr = ManagementCanister::create(env.get_agent());
info!(
env.get_logger(),
"Creating UI canister on the {} network.", network.name
Expand Down
Loading

0 comments on commit d8cc522

Please sign in to comment.