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.

Sign up for GitHub

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

feat: withdraw cycles to cycles ledger #3506

Merged
merged 6 commits into from
Jan 17, 2024
Merged

feat: withdraw cycles to cycles ledger #3506

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8155cbd
feat: withdraw cycles to cycles ledger
sesi200 Jan 16, 2024
2132a25
shellcheck
sesi200 Jan 16, 2024
af34ccf
fix deps tests
sesi200 Jan 16, 2024
7d96ad0
Merge branch 'master' into SDK-1325-withdraw-cycles-from-deleted-cani…
sesi200 Jan 17, 2024
dbd94c3
turn feature flag off
sesi200 Jan 17, 2024
a40e1bb
forgot to skip one test...
sesi200 Jan 17, 2024
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
44 changes: 44 additions & 0 deletions e2e/tests-dfx/cycles-ledger.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,7 @@ current_time_nanoseconds() {

# using dfx canister create
dfx identity use alice
# shellcheck disable=SC2030
export DFX_DISABLE_AUTO_WALLET=1
t=$(current_time_nanoseconds)
assert_command dfx canister create e2e_project_backend --with-cycles 1T --created-at-time "$t"
Expand Down Expand Up @@ -501,3 +502,46 @@ current_time_nanoseconds() {
assert_command dfx cycles balance --precise
assert_eq "9399600000000 cycles."
}

@test "canister deletion" {
# skip "can't be properly tested with feature flag turned off (CYCLES_LEDGER_ENABLED). TODO(SDK-1331): re-enable this test"
dfx_new temporary
add_cycles_ledger_canisters_to_project
install_cycles_ledger_canisters

ALICE=$(dfx identity get-principal --identity alice)

assert_command deploy_cycles_ledger
CYCLES_LEDGER_ID=$(dfx canister id cycles-ledger)
echo "Cycles ledger deployed at id $CYCLES_LEDGER_ID"
assert_command dfx deploy cycles-depositor --argument "(record {ledger_id = principal \"$(dfx canister id cycles-ledger)\"})"
echo "Cycles depositor deployed at id $(dfx canister id cycles-depositor)"
assert_command dfx ledger fabricate-cycles --canister cycles-depositor --t 9999
assert_command dfx deploy
assert_command dfx canister call cycles-depositor deposit "(record {to = record{owner = principal \"$ALICE\";};cycles = 22_400_000_000_000;})" --identity cycle-giver

cd ..
dfx_new
# setup done

dfx identity use alice
# shellcheck disable=SC2031
export DFX_DISABLE_AUTO_WALLET=1
assert_command dfx canister create --all --with-cycles 10T
assert_command dfx cycles balance --precise
assert_eq "2399800000000 cycles."

# delete by name
assert_command dfx canister stop --all
assert_command dfx canister delete e2e_project_backend
assert_command dfx cycles balance
assert_eq "12.389 TC (trillion cycles)."

# delete by id
FRONTEND_ID=$(dfx canister id e2e_project_frontend)
rm .dfx/local/canister_ids.json
assert_command dfx canister stop "${FRONTEND_ID}"
assert_command dfx canister delete "${FRONTEND_ID}"
assert_command dfx cycles balance
assert_eq "22.379 TC (trillion cycles)."
}
24 changes: 12 additions & 12 deletions e2e/tests-dfx/deps.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ Failed to download from url: http://example.com/c.wasm."

cd ../onchain
dfx canister stop a
dfx canister delete a
dfx canister delete a --no-withdrawal

cd ../app
assert_command_fail dfx deps pull --network local
Expand Down Expand Up @@ -325,11 +325,11 @@ candid:args => (nat)"
# delete onchain canisters so that the replica has no canisters as a clean local replica
cd ../onchain
dfx canister stop a
dfx canister delete a
dfx canister delete a --no-withdrawal
dfx canister stop b
dfx canister delete b
dfx canister delete b --no-withdrawal
dfx canister stop c
dfx canister delete c
dfx canister delete c --no-withdrawal

cd ../app
assert_command dfx deps init # b is set here
Expand All @@ -355,10 +355,10 @@ Installing canister: $CANISTER_ID_C (dep_c)"

# deployed pull dependencies can be stopped and deleted
assert_command dfx canister stop dep_b --identity anonymous
assert_command dfx canister delete dep_b --identity anonymous
assert_command dfx canister delete dep_b --identity anonymous --no-withdrawal

assert_command dfx canister stop $CANISTER_ID_A --identity anonymous
assert_command dfx canister delete $CANISTER_ID_A --identity anonymous
assert_command dfx canister delete $CANISTER_ID_A --identity anonymous --no-withdrawal

# error cases
## set wrong init argument
Expand Down Expand Up @@ -397,11 +397,11 @@ Installing canister: $CANISTER_ID_C (dep_c)"
# delete onchain canisters so that the replica has no canisters as a clean local replica
cd ../onchain
dfx canister stop a
dfx canister delete a
dfx canister delete a --no-withdrawal
dfx canister stop b
dfx canister delete b
dfx canister delete b --no-withdrawal
dfx canister stop c
dfx canister delete c
dfx canister delete c --no-withdrawal

cd ../app
assert_command_fail dfx canister create dep_b
Expand Down Expand Up @@ -434,7 +434,7 @@ Installing canister: $CANISTER_ID_C (dep_c)"

# start a clean local replica
dfx canister stop app
dfx canister delete app
dfx canister delete app --no-withdrawal
assert_command dfx deploy # only deploy app canister
}

Expand All @@ -443,8 +443,8 @@ Installing canister: $CANISTER_ID_C (dep_c)"

# verify the help message
assert_command dfx deps pull -h
assert_contains "Pull canisters upon which the project depends. This command connects to the \"ic\" mainnet by default.
You can still choose other network by setting \`--network\`"
assert_contains "Pull canisters upon which the project depends. This command connects to the \"ic\" mainnet by default."
assert_contains "You can still choose other network by setting \`--network\`"

assert_command dfx deps pull
assert_contains "There are no pull dependencies defined in dfx.json"
Expand Down
137 changes: 94 additions & 43 deletions src/dfx/src/commands/canister/delete.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@ use crate::lib::operations::canister;
use crate::lib::operations::canister::{
deposit_cycles, start_canister, stop_canister, update_settings,
};
use crate::lib::operations::cycles_ledger::{
wallet_deposit_to_cycles_ledger, CYCLES_LEDGER_ENABLED,
};
use crate::lib::root_key::fetch_root_key_if_needed;
use crate::util::assets::wallet_wasm;
use crate::util::blob_from_arguments;
use anyhow::Context;
use crate::util::clap::parsers::icrc_subaccount_parser;
use anyhow::{bail, Context};
use candid::Principal;
use clap::Parser;
use dfx_core::canister::build_wallet_canister;
Expand All @@ -23,6 +27,7 @@ use ic_utils::interfaces::management_canister::builders::InstallMode;
use ic_utils::interfaces::management_canister::CanisterStatus;
use ic_utils::interfaces::ManagementCanister;
use ic_utils::Argument;
use icrc_ledger_types::icrc1::account::{Account, Subaccount};
use num_traits::cast::ToPrimitive;
use slog::info;
use std::convert::TryFrom;
Expand Down Expand Up @@ -71,6 +76,11 @@ pub struct CanisterDeleteOpts {
/// Auto-confirm deletion for a non-stopped canister.
#[arg(long, short)]
yes: bool,

/// Subaccount of the selected identity to deposit cycles to.
//TODO(SDK-1331): unhide
#[arg(long, value_parser = icrc_subaccount_parser, hide = true)]
to_subaccount: Option<Subaccount>,
}

#[context("Failed to delete canister '{}'.", canister)]
Expand All @@ -83,6 +93,7 @@ async fn delete_canister(
withdraw_cycles_to_canister: Option<String>,
withdraw_cycles_to_dank: bool,
withdraw_cycles_to_dank_principal: Option<String>,
to_cycles_ledger_subaccount: Option<Subaccount>,
) -> DfxResult {
let log = env.get_logger();
let mut canister_id_store = env.get_canister_id_store()?;
Expand All @@ -99,19 +110,23 @@ async fn delete_canister(
let to_dank = withdraw_cycles_to_dank || withdraw_cycles_to_dank_principal.is_some();

// Get the canister to transfer the cycles to.
let target_canister_id = if no_withdrawal {
None
let withdraw_target = if no_withdrawal {
WithdrawTarget::NoWithdrawal
} else if to_dank {
Some(DANK_PRINCIPAL)
WithdrawTarget::Dank
} else {
match withdraw_cycles_to_canister {
Some(ref target_canister_id) => {
Some(Principal::from_text(target_canister_id).with_context(|| {
format!("Failed to read canister id {:?}.", target_canister_id)
})?)
let canister_id =
Principal::from_text(target_canister_id).with_context(|| {
format!("Failed to read canister id {:?}.", target_canister_id)
})?;
WithdrawTarget::Canister { canister_id }
}
None => match call_sender {
CallSender::Wallet(wallet_id) => Some(*wallet_id),
CallSender::Wallet(wallet_id) => WithdrawTarget::Canister {
canister_id: *wallet_id,
},
CallSender::SelectedId => {
let network = env.get_network_descriptor();
let agent_env = create_agent_environment(env, Some(network.name.clone()))?;
Expand All @@ -120,7 +135,19 @@ async fn delete_canister(
.expect("No selected identity.")
.to_string();
// If there is no wallet, then do not attempt to withdraw the cycles.
wallet_canister_id(network, &identity_name)?
match wallet_canister_id(network, &identity_name)? {
Some(canister_id) => WithdrawTarget::Canister { canister_id },
None if CYCLES_LEDGER_ENABLED => {
let Some(my_principal) = env.get_selected_identity_principal() else { bail!("Identity has no principal attached") };
WithdrawTarget::CyclesLedger {
to: Account {
owner: my_principal,
subaccount: to_cycles_ledger_subaccount,
},
}
}
_ => WithdrawTarget::NoWithdrawal,
}
}
},
}
Expand All @@ -135,11 +162,10 @@ async fn delete_canister(
};
fetch_root_key_if_needed(env).await?;

if let Some(target_canister_id) = target_canister_id {
if withdraw_target != WithdrawTarget::NoWithdrawal {
info!(
log,
"Beginning withdrawal of cycles to canister {}; on failure try --no-wallet --no-withdrawal.",
target_canister_id
"Beginning withdrawal of cycles; on failure try --no-wallet --no-withdrawal."
);

// Determine how many cycles we can withdraw.
Expand Down Expand Up @@ -197,40 +223,55 @@ async fn delete_canister(
break;
}
let cycles_to_withdraw = cycles - margin;
let result = if !to_dank {
info!(
log,
"Attempting to transfer {} cycles to canister {}.",
cycles_to_withdraw,
target_canister_id
);
// Transfer cycles from the source canister to the target canister using the temporary wallet.
deposit_cycles(
env,
target_canister_id,
&CallSender::Wallet(canister_id),
cycles_to_withdraw,
)
.await
} else {
info!(
log,
"Attempting to transfer {} cycles to dank principal {}.",
cycles_to_withdraw,
dank_target_principal
);
let wallet = build_wallet_canister(canister_id, agent).await?;
let opt_principal = Some(dank_target_principal);
wallet
.call(
let result = match withdraw_target {
WithdrawTarget::NoWithdrawal => Ok(()),
WithdrawTarget::Dank => {
info!(
log,
"Attempting to transfer {} cycles to dank principal {}.",
cycles_to_withdraw,
dank_target_principal
);
let wallet = build_wallet_canister(canister_id, agent).await?;
let opt_principal = Some(dank_target_principal);
wallet
.call(
DANK_PRINCIPAL,
"mint",
Argument::from_candid((opt_principal,)),
cycles_to_withdraw,
)
.call_and_wait()
.await
.context("Failed mint call.")
}
WithdrawTarget::Canister {
canister_id: target_canister_id,
} => {
info!(
log,
"Attempting to transfer {} cycles to canister {}.",
cycles_to_withdraw,
target_canister_id
);
// Transfer cycles from the source canister to the target canister using the temporary wallet.
deposit_cycles(
env,
target_canister_id,
"mint",
Argument::from_candid((opt_principal,)),
&CallSender::Wallet(canister_id),
cycles_to_withdraw,
)
.call_and_wait()
.await
.context("Failed mint call.")
}
WithdrawTarget::CyclesLedger { to } => {
wallet_deposit_to_cycles_ledger(
agent,
canister_id,
cycles_to_withdraw,
to,
)
.await
}
};
if result.is_ok() {
info!(log, "Successfully withdrew {} cycles.", cycles_to_withdraw);
Expand All @@ -239,7 +280,7 @@ async fn delete_canister(
info!(log, "Not enough margin. Trying again with more margin.");
attempts += 1;
} else {
// Unforseen error. Report it back to user
// Unforeseen error. Report it back to user
result?;
}
}
Expand Down Expand Up @@ -293,6 +334,7 @@ pub async fn exec(
opts.withdraw_cycles_to_canister,
opts.withdraw_cycles_to_dank,
opts.withdraw_cycles_to_dank_principal,
opts.to_subaccount,
)
.await
} else if opts.all {
Expand All @@ -307,6 +349,7 @@ pub async fn exec(
opts.withdraw_cycles_to_canister.clone(),
opts.withdraw_cycles_to_dank,
opts.withdraw_cycles_to_dank_principal.clone(),
opts.to_subaccount,
)
.await?;
}
Expand All @@ -316,3 +359,11 @@ pub async fn exec(
unreachable!()
}
}

#[derive(Debug, Clone, Copy, Eq, PartialEq)]
enum WithdrawTarget {
NoWithdrawal,
Dank,
CyclesLedger { to: Account },
Canister { canister_id: Principal },
}
Loading
Loading