Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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 @@ -6,6 +6,7 @@
* the wallet surface is unavailable.
*/
import { RefusedBeforeWrite } from "@bramble/core/errors";
import { NO_RECOVERY_PHRASE_REFUSAL } from "@bramble/wallet-model/accounts";
import { DEFAULT_NETWORK } from "@bramble/wallet-model/networks";
import {
readCreateAccountInput,
Expand Down Expand Up @@ -51,16 +52,13 @@ const handleCreateAccount: RouterHandler = async (ctx, messageType, message) =>
});
};

// The refusal `addDerivedAccount` raises for a vault with no phrase, verbatim.
const NO_RECOVERY_PHRASE = "No recovery phrase stored for this wallet";

const missingPhraseRefusal = async (ctx: RouterContext): Promise<RuntimeResponse | null> => {
const passphrase = ctx.session.peekPassphrase();
if (!passphrase) {
return { error: "wallet locked" };
}
const vault = await ctx.wallet.readVault(passphrase);
return vault.mnemonic ? null : { error: NO_RECOVERY_PHRASE };
return vault.mnemonic ? null : { error: NO_RECOVERY_PHRASE_REFUSAL };
};

const handleAddAccount: RouterHandler = async (ctx, messageType, message) => {
Expand Down
7 changes: 7 additions & 0 deletions packages/foundation/wallet-model/src/accounts/accounts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -490,3 +490,10 @@ export const canonicalLegacyAccountRef = (value: unknown): LegacyAccountRef | nu
*/
export const isKeylessAccount = (account: AccountRef): account is LedgerAccountRef =>
account.derivation.kind === "ledger";

/**
* Refusal for deriving an account in a vault set up from a Ledger or a pasted
* key. One string for the vault, the background handler and the panel, which
* routes on it to import instead of an error page (#740).
*/
export const NO_RECOVERY_PHRASE_REFUSAL = "No recovery phrase stored for this wallet";
4 changes: 2 additions & 2 deletions packages/platform/vault/src/service/service-creation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*/

import { RefusedBeforeWrite, WalletError } from "@bramble/core/errors";
import type { DerivedAccountRef } from "@bramble/wallet-model/accounts";
import { type DerivedAccountRef, NO_RECOVERY_PHRASE_REFUSAL } from "@bramble/wallet-model/accounts";
import type { StarknetNetworkId } from "@bramble/wallet-model/networks";
import {
deriveAccount,
Expand Down Expand Up @@ -102,7 +102,7 @@ export const vaultCreationMethods = ({
async addDerivedAccount(input) {
const vault = await readVaultSnapshot(input.vaultPassphrase);
if (!vault.mnemonic) {
throw new RefusedBeforeWrite("No recovery phrase stored for this wallet");
throw new RefusedBeforeWrite(NO_RECOVERY_PHRASE_REFUSAL);
}
const { account, privateKey } = await deriveNextFreeAccount(kms, vault, {
accountContractType: input.accountContractType,
Expand Down
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 @@ -24,6 +24,7 @@

import { NETWORK_RETRY_MESSAGE } from "@bramble/core/network-error";
import { WRONG_CHAIN_MESSAGE } from "@bramble/starknet/provider";
import { NO_RECOVERY_PHRASE_REFUSAL } from "@bramble/wallet-model/accounts";

import { MULTISIG_ERROR_COPY } from "./friendly-error-copy-multisig";
import { PRIVACY_ERROR_COPY } from "./friendly-error-copy-privacy";
Expand Down Expand Up @@ -249,7 +250,7 @@ export const ERROR_COPY: ReadonlyMap<string, string> = new Map([
"This wallet was set up without a recovery phrase, so there is nothing to search for.",
],
[
"no recovery phrase stored for this wallet",
NO_RECOVERY_PHRASE_REFUSAL.toLowerCase(),
"This wallet was set up without a recovery phrase. Import an existing account instead.",
],
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
* the reader cannot diagnose gets the same sentence wherever it comes from.
*/

import { NO_RECOVERY_PHRASE_REFUSAL } from "@bramble/wallet-model/accounts";

/** Shared ending for a change that did not persist. */
export const SAVE_FAILED = "Couldn't save that change. Try again.";

Expand All @@ -17,6 +19,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_REFUSAL.toLowerCase();

/** 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"');
});
});
31 changes: 31 additions & 0 deletions tests/background/accounts/add-account-no-phrase.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import { createWalletVaultService } from "@bramble/vault/service";
import { createWalletReader } from "../../../apps/extension/src/background/accounts/wallet-reader";
import { createMessageHandler } from "../../../apps/extension/src/background/routing/messages";
import { createUnlockSession } from "../../../apps/extension/src/background/runtime/session";
import { friendlyError } from "../../../packages/presentation/wallet-ui/src/sidepanel/runtime/friendly-error";
import { isPhraselessAddRefusal } from "../../../packages/presentation/wallet-ui/src/sidepanel/runtime/friendly-error-shared";
import {
extensionSender as sender,
testExtension,
Expand Down Expand Up @@ -89,4 +91,33 @@ describe("wallet.addAccount without a recovery phrase", () => {
}),
).rejects.toThrow("No recovery phrase stored for this wallet");
});

// The panel routes a phrase-less wallet's "Regular account" choice to import
// only when it recognises this refusal (#740). Both sources must keep matching
// it, or the choice falls back to an error page.
test("sends a refusal the panel routes to import, from the handler and the vault", async () => {
const { handler, service } = await ledgerWallet();
const response = await handler(
{ accountContractType: "openzeppelin", networkId: "SN_MAIN", type: "wallet.addAccount" },
sender,
);
const handlerRefusal = (response as { readonly error: string }).error;
const vaultRefusal = await service
.addDerivedAccount({
accountContractType: "openzeppelin",
networkId: "SN_MAIN",
vaultPassphrase,
})
.then(
() => "",
(error: unknown) => (error as Error).message,
);

for (const refusal of [handlerRefusal, vaultRefusal]) {
expect(isPhraselessAddRefusal(refusal)).toBe(true);
expect(friendlyError(refusal).message).toBe(
"This wallet was set up without a recovery phrase. Import an existing account instead.",
);
}
});
});
Loading