Skip to content

Commit

Permalink
feat: improve missing ledger error message (#3995)
Browse files Browse the repository at this point in the history
  • Loading branch information
sesi200 authored Nov 22, 2024
1 parent 1c22db8 commit 8cb62ce
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 7 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
30 changes: 25 additions & 5 deletions e2e/tests-dfx/ledger.bash
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,20 @@

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
install_shared_asset subnet_type/shared_network_settings/system

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() {
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -269,6 +281,7 @@ tc_to_num() {
}

@test "ledger show-subnet-types" {
install_nns
install_asset cmc

dfx deploy cmc
Expand All @@ -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."
}
16 changes: 16 additions & 0 deletions src/dfx/src/lib/diagnosis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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()))
}

0 comments on commit 8cb62ce

Please sign in to comment.