From 8cb62ce7afa65a7de23640843a5c69d349f255b6 Mon Sep 17 00:00:00 2001
From: Severin Siffert <severin.siffert@dfinity.org>
Date: Fri, 22 Nov 2024 16:30:02 +0100
Subject: [PATCH] feat: improve missing ledger error message (#3995)

---
 CHANGELOG.md                 |  4 ++--
 e2e/tests-dfx/ledger.bash    | 30 +++++++++++++++++++++++++-----
 src/dfx/src/lib/diagnosis.rs | 16 ++++++++++++++++
 3 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a3ed50fef1..d76bb09abb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -41,9 +41,9 @@ Allow setting permissions lists in init arguments just like in upgrade arguments
 - Module hash: f45db224b40fac516c877e3108dc809d4b22fa42d05ee8dfa5002536a3a3daed
 - Bump agent-js to fix error code
 
-### chore!: improve the messages for the subcommands of `dfx cycles`.
+### chore!: improve the messages for the subcommands of `dfx cycles` and `dfx ledger`.
 
-If users run subcommands of `dfx cycles` without the `--ic` flag, show below messages to indicate what to do next.
+If users run subcommands of `dfx cycles` or `dfx ledger` without the `--ic` flag, show below messages to indicate what to do next.
 ```
 Error explanation:
 Cycles ledger with canister ID 'um5iw-rqaaa-aaaaq-qaaba-cai' is not installed.
diff --git a/e2e/tests-dfx/ledger.bash b/e2e/tests-dfx/ledger.bash
index f41100b847..ea5fa40567 100644
--- a/e2e/tests-dfx/ledger.bash
+++ b/e2e/tests-dfx/ledger.bash
@@ -2,6 +2,13 @@
 
 load ../utils/_
 
+install_nns() {
+  dfx_start_for_nns_install
+
+  dfx extension install nns --version 0.4.3
+  dfx nns install --ledger-accounts 345f723e9e619934daac6ae0f4be13a7b0ba57d6a608e511a00fd0ded5866752 22ca7edac648b814e81d7946e8bacea99280e07c5f51a04ba7a38009d8ad8e89 5a94fe181e9d411c58726cb87cbf2d016241b6c350bc3330e4869ca76e54ecbc
+}
+
 setup() {
   standard_setup
   install_asset ledger
@@ -9,11 +16,6 @@ setup() {
 
   dfx identity import --storage-mode plaintext alice alice.pem
   dfx identity import --storage-mode plaintext bob bob.pem
-
-  dfx_start_for_nns_install
-
-  dfx extension install nns --version 0.4.3
-  dfx nns install --ledger-accounts 345f723e9e619934daac6ae0f4be13a7b0ba57d6a608e511a00fd0ded5866752 22ca7edac648b814e81d7946e8bacea99280e07c5f51a04ba7a38009d8ad8e89 5a94fe181e9d411c58726cb87cbf2d016241b6c350bc3330e4869ca76e54ecbc
 }
 
 teardown() {
@@ -27,6 +29,8 @@ current_time_nanoseconds() {
 }
 
 @test "ledger account-id" {
+  install_nns
+
   dfx identity use alice
   assert_command dfx ledger account-id
   assert_match 345f723e9e619934daac6ae0f4be13a7b0ba57d6a608e511a00fd0ded5866752
@@ -46,6 +50,8 @@ current_time_nanoseconds() {
 }
 
 @test "ledger balance & transfer" {
+  install_nns
+
   dfx identity use alice
   assert_command dfx ledger account-id
   assert_eq 345f723e9e619934daac6ae0f4be13a7b0ba57d6a608e511a00fd0ded5866752
@@ -104,6 +110,8 @@ current_time_nanoseconds() {
 }
 
 @test "ledger subaccounts" {
+  install_nns
+
   subacct=000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f
   assert_command dfx ledger account-id --identity bob --subaccount "$subacct"
   assert_match 5a94fe181e9d411c58726cb87cbf2d016241b6c350bc3330e4869ca76e54ecbc
@@ -140,6 +148,8 @@ tc_to_num() {
 }
 
 @test "ledger top-up" {
+  install_nns
+
   dfx identity use alice
   assert_command dfx ledger balance
   assert_match "1000000000.00000000 ICP"
@@ -198,6 +208,8 @@ tc_to_num() {
 }
 
 @test "ledger create-canister" {
+  install_nns
+
   dfx identity use alice
   assert_command dfx ledger create-canister --amount=100 --subnet-type "type1" "$(dfx identity get-principal)"
   assert_match "Transfer sent at block height"
@@ -269,6 +281,7 @@ tc_to_num() {
 }
 
 @test "ledger show-subnet-types" {
+  install_nns
   install_asset cmc
 
   dfx deploy cmc
@@ -278,3 +291,10 @@ tc_to_num() {
   assert_command dfx ledger show-subnet-types --cycles-minting-canister-id "$CANISTER_ID"
   assert_eq '["type1", "type2"]'
 }
+
+@test "balance without ledger fails as expected" {
+  dfx_start
+
+  assert_command_fail dfx ledger balance
+  assert_contains "ICP Ledger with canister ID 'ryjl3-tyaaa-aaaaa-aaaba-cai' is not installed."
+}
diff --git a/src/dfx/src/lib/diagnosis.rs b/src/dfx/src/lib/diagnosis.rs
index e59a810da7..150f1c9b56 100644
--- a/src/dfx/src/lib/diagnosis.rs
+++ b/src/dfx/src/lib/diagnosis.rs
@@ -57,6 +57,9 @@ pub fn diagnose(err: &AnyhowError) -> Diagnosis {
         if cycles_ledger_not_found(err) {
             return diagnose_cycles_ledger_not_found();
         }
+        if ledger_not_found(err) {
+            return diagnose_ledger_not_found();
+        }
     }
 
     if local_replica_not_running(err) {
@@ -246,3 +249,16 @@ fn diagnose_cycles_ledger_not_found() -> Diagnosis {
 
     (Some(explanation.to_string()), Some(suggestion.to_string()))
 }
+
+fn ledger_not_found(err: &AnyhowError) -> bool {
+    err.to_string()
+        .contains("Canister ryjl3-tyaaa-aaaaa-aaaba-cai not found")
+}
+
+fn diagnose_ledger_not_found() -> Diagnosis {
+    let explanation = "ICP Ledger with canister ID 'ryjl3-tyaaa-aaaaa-aaaba-cai' is not installed.";
+    let suggestion =
+        "Run the command with '--ic' flag if you want to manage the ICP on the mainnet.";
+
+    (Some(explanation.to_string()), Some(suggestion.to_string()))
+}