Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ export const createLedgerOnboardRuntime = (
return addAccountChooser(ledgerAvailable);
}
if (view.kind === "import-chooser") {
return importAccountChooser(ledgerAvailable);
return importAccountChooser(ledgerAvailable, view.phraselessNote);
}
if (view.kind === "connect") {
return ledgerConnectPrompt();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import {
READY_EIP2645_APPLICATION,
} from "@bramble/wallet-model/ledger-path";
import { friendlyError } from "../../runtime/friendly-error";
import { ADD_ACCOUNT_FAILED } from "../../runtime/friendly-error-shared";
import { ADD_ACCOUNT_FAILED, isPhraselessAddRefusal } from "../../runtime/friendly-error-shared";
import {
type LedgerCandidate,
type LedgerCandidateState,
Expand Down Expand Up @@ -405,11 +405,15 @@ export const createLedgerOnboardController = (
deps.render(null);
await deps.refresh();
} catch (error) {
const message = friendlyError(
error instanceof Error ? error.message : "",
ADD_ACCOUNT_FAILED,
).message;
deps.render({ kind: "error", message });
const raw = error instanceof Error ? error.message : "";
const message = friendlyError(raw, ADD_ACCOUNT_FAILED).message;
// A phrase-less wallet can't derive a regular account, but its import
// routes still work: land there rather than on a Ledger "Try again".
deps.render(
isPhraselessAddRefusal(raw)
? { kind: "import-chooser", phraselessNote: message }
: { kind: "error", message },
);
} finally {
busy = false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export type LedgerOnboardAddKind =
*/
export type LedgerOnboardView =
| { readonly kind: "chooser" }
| { readonly kind: "import-chooser" }
| { readonly kind: "import-chooser"; readonly phraselessNote?: string }
| { readonly kind: "connect" }
| { readonly kind: "devices"; readonly devices: readonly LedgerDeviceChoice[] }
| { readonly kind: "reading" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,11 @@ export const addAccountChooser = (ledgerAvailable = true): string => {
* that create one. The plain Ledger route stays on the chooser above, since it
* also creates accounts on Bramble's own derivation path. `ledgerAvailable`
* false omits the Ready Ledger route on a runtime that cannot reach a device.
* `phraselessNote` is set when a wallet with no recovery phrase was sent here
* from "Regular account": it says why, and the rescan, which needs that
* phrase, is left out.
*/
export const importAccountChooser = (ledgerAvailable = true): string => {
export const importAccountChooser = (ledgerAvailable = true, phraselessNote?: string): string => {
return `
<div class="onboard-hero add-account-hero">
<header class="add-account-header">
Expand All @@ -115,8 +118,9 @@ export const importAccountChooser = (ledgerAvailable = true): string => {
variant: "ghost",
})}
</header>
${phraselessNote ? `<p class="hint" role="status">${escapeHtml(phraselessNote)}</p>` : ""}
<div class="add-account-chooser" role="group" aria-label="Import an existing account">
${chooserOption("find-accounts", "users", "Find my other accounts", "Search this wallet for accounts it does not list yet.")}
${phraselessNote ? "" : chooserOption("find-accounts", "users", "Find my other accounts", "Search this wallet for accounts it does not list yet.")}
${chooserOption("import-phrase", "book", "Recovery phrase", "Import accounts from another wallet's 12 or 24 words.")}
${chooserOption("import-key", "link", "Private key", "Import a single account from its raw Starknet key.")}
${ledgerAvailable ? chooserOption("restore-ready-ledger", "shield", "Ready Ledger account", "Bring over a Ledger account created in Ready (Argent X).") : ""}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ export const SUBMISSION_UNKNOWN = (what: string): string =>
/** Shared ending for an account that was not added (add, create, or import). */
export const ADD_ACCOUNT_FAILED = "Couldn't add that account. Try again.";

/**
* True for the refusal `wallet.addAccount` returns when the vault was set up
* from a Ledger or a pasted key and so holds no phrase to derive from.
*/
export const isPhraselessAddRefusal = (raw: string): boolean =>
raw.trim().toLowerCase() === "no recovery phrase stored for this wallet";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the third copy of this literal (the vault's addDerivedAccount and NO_RECOVERY_PHRASE in account-creation-handlers.ts are the others). If one is reworded, this silently goes back to the error page. That's safe, but #740 would come back. Could the background export the constant, or return a reason code, so a test pins the two ends together?


/** Shared ending for a recovery phrase the wallet could not import. */
export const SEED_IMPORT_FAILED =
"Couldn't import that recovery phrase. Check the words and try again.";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,4 +182,22 @@ describe("add-account chooser routing", () => {
"The wallet is locked or unavailable. Unlock it and try again.",
);
});

// Issue 740: a wallet set up from a Ledger or a pasted key holds no phrase,
// so the regular tile lands on the import routes that still work, not on an
// error page whose "Try again" reconnects a Ledger.
test("a phrase-less wallet's regular-account choice lands on import, not an error", async () => {
const h = onboardHarness({
addHd: () => Promise.reject(new Error("No recovery phrase stored for this wallet")),
});
const controller = createLedgerOnboardController(h.deps);
controller.showChooser();
controller.choose("regular");
await tick();
expect(h.views.at(-1)).toEqual({
kind: "import-chooser",
phraselessNote:
"This wallet was set up without a recovery phrase. Import an existing account instead.",
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -378,4 +378,19 @@ describe("Ledger onboarding runtime (DOM)", () => {
h.runtime.startAddAccount();
expect(h.mount.querySelector('[data-add-kind="ledger"]')).not.toBeNull();
});

test("a phrase-less wallet's regular tile renders the import routes with the reason", async () => {
const h = mountRuntime({
addHd: () => Promise.reject(new Error("No recovery phrase stored for this wallet")),
});
h.runtime.startAddAccount();
click(h.mount, '[data-add-kind="regular"]');
await tick();
expect(h.mount.querySelector('[role="status"]')?.textContent).toBe(
"This wallet was set up without a recovery phrase. Import an existing account instead.",
);
expect(h.mount.querySelector('[data-add-kind="find-accounts"]')).toBeNull();
expect(h.mount.querySelector('[data-add-kind="import-phrase"]')).not.toBeNull();
expect(h.mount.querySelector("[data-ledger-onboard-connect]")).toBeNull();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -292,4 +292,16 @@ describe("onboarding views", () => {
expect(importChooser).toContain('data-add-kind="import-key"');
expect((importChooser.match(/data-add-kind="/g) ?? []).length).toBe(3);
});

// Issue 740: the note says why a phrase-less wallet landed here, and the
// rescan, which needs the phrase it lacks, is left out.
test("the import chooser for a phrase-less wallet explains itself and drops the rescan", () => {
const html = importAccountChooser(true, "No <phrase> here.");
expect(html).toContain('role="status">No &lt;phrase&gt; here.</p>');
expect(html).not.toContain('data-add-kind="find-accounts"');
expect(html).toContain('data-add-kind="import-phrase"');
expect(html).toContain('data-add-kind="import-key"');
expect(html).toContain('data-add-kind="restore-ready-ledger"');
expect(importAccountChooser()).not.toContain('role="status"');
});
});
Loading