diff --git a/CHANGELOG.md b/CHANGELOG.md index 08f5ab11d8..24d347c86f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,22 @@ Please top up your cycles balance by converting ICP to cycles like below: 'dfx cycles convert --amount=0.123'. ``` +### chore: improve `dfx cycles convert` messages. + +If users run `dfx cycles convert` without enough ICP tokens, show additional messages to indicate what to do next. +``` +Error explanation: +Insufficient ICP balance to finish the transfer transaction. +How to resolve the error: +Please top up your ICP balance. + +Your account address for receiving ICP from centralized exchanges: 8494c01329531c06254ff45dad87db806ae6ed935ad6a504cdbc00a935db7b49 +(run `dfx ledger account-id` to display) + +Your principal for ICP wallets and decentralized exchanges: ueuar-wxbnk-bdcsr-dnrh3-rsyq6-ffned-h64ox-vxywi-gzawf-ot4pv-sqe +(run `dfx identity get-principal` to display) +``` + # 0.24.3 ### feat: Bitcoin support in PocketIC diff --git a/e2e/tests-dfx/cycles-ledger.bash b/e2e/tests-dfx/cycles-ledger.bash index 83fa0ddd99..37dea54dd8 100644 --- a/e2e/tests-dfx/cycles-ledger.bash +++ b/e2e/tests-dfx/cycles-ledger.bash @@ -898,6 +898,10 @@ current_time_nanoseconds() { deploy_cycles_ledger + # Test failed to convert ICP to cycles without enough ICP. + assert_command_fail dfx cycles convert --amount 12.5 --identity alice + assert_contains "Insufficient ICP balance to finish the transfer transaction." + assert_command dfx --identity cycle-giver ledger transfer --memo 1234 --amount 100 "$(dfx ledger account-id --of-principal "$ALICE")" assert_command dfx --identity cycle-giver ledger transfer --memo 1234 --amount 100 "$(dfx ledger account-id --of-principal "$ALICE" --subaccount "$ALICE_SUBACCT1")" diff --git a/src/dfx/src/lib/diagnosis.rs b/src/dfx/src/lib/diagnosis.rs index b346341cf7..49bd3b2b77 100644 --- a/src/dfx/src/lib/diagnosis.rs +++ b/src/dfx/src/lib/diagnosis.rs @@ -17,8 +17,6 @@ pub type Diagnosis = (Option, Option); pub const NULL_DIAGNOSIS: Diagnosis = (None, None); #[derive(ThisError, Debug)] -// This message will appear in the context trace of the stack. The diagnosis should not be displayed there yet. -#[error("Diagnosis was added here.")] /// If you do not need the generic error diagnosis to run, you can add a DiagnosedError with .context(err: DiagnosedError). /// In that case, no extra diagnosis is attempted and the last-added explanation and suggestion are printed out. pub struct DiagnosedError { @@ -29,6 +27,12 @@ pub struct DiagnosedError { pub action_suggestion: Option, } +impl std::fmt::Display for DiagnosedError { + fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + Ok(()) + } +} + impl DiagnosedError { pub fn new(error_explanation: String, action_suggestion: String) -> Self { Self { diff --git a/src/dfx/src/lib/operations/ledger.rs b/src/dfx/src/lib/operations/ledger.rs index d48047ea66..37f882ea79 100644 --- a/src/dfx/src/lib/operations/ledger.rs +++ b/src/dfx/src/lib/operations/ledger.rs @@ -1,3 +1,4 @@ +use crate::lib::diagnosis::DiagnosedError; use crate::lib::ledger_types::{AccountIdBlob, BlockHeight, Memo, TransferError}; use crate::lib::nns_types::account_identifier::Subaccount; use crate::lib::{ @@ -8,7 +9,7 @@ use crate::lib::{ }, nns_types::{account_identifier::AccountIdentifier, icpts::ICPTs}, }; -use anyhow::{bail, ensure, Context}; +use anyhow::{anyhow, bail, ensure, Context}; use backoff::backoff::Backoff; use backoff::ExponentialBackoff; use candid::{Decode, Encode, Principal}; @@ -142,6 +143,12 @@ pub async fn transfer( info!(logger, "{}", TransferError::TxDuplicate { duplicate_of }); break duplicate_of; } + Err(TransferError::InsufficientFunds { balance }) => { + return Err(anyhow!(TransferError::InsufficientFunds { balance })) + .with_context(|| { + diagnose_insufficient_funds_error(agent, from_subaccount) + }); + } Err(transfer_err) => bail!(transfer_err), } } @@ -164,6 +171,29 @@ pub async fn transfer( Ok(block_height) } +fn diagnose_insufficient_funds_error( + agent: &Agent, + subaccount: Option, +) -> DiagnosedError { + let principal = agent.get_principal().unwrap(); // This should always succeed at this point. + + let explanation = "Insufficient ICP balance to finish the transfer transaction."; + let suggestion = format!( + "Please top up your ICP balance. + +Your account address for receiving ICP from centralized exchanges: {} +(run `dfx ledger account-id` to display) + +Your principal for ICP wallets and decentralized exchanges: {} +(run `dfx identity get-principal` to display) +", + AccountIdentifier::new(principal, subaccount), + principal.to_text() + ); + + DiagnosedError::new(explanation.to_string(), suggestion) +} + fn retryable(agent_error: &AgentError) -> bool { match agent_error { AgentError::CertifiedReject(RejectResponse { diff --git a/src/dfx/src/main.rs b/src/dfx/src/main.rs index 79a19d3542..85603b4657 100644 --- a/src/dfx/src/main.rs +++ b/src/dfx/src/main.rs @@ -72,6 +72,10 @@ fn print_error_and_diagnosis(err: Error, error_diagnosis: Diagnosis) { // print error chain stack for (level, cause) in err.chain().enumerate() { + if cause.to_string().is_empty() { + continue; + } + let (color, prefix) = if level == 0 { (term::color::RED, "Error") } else {