diff --git a/README.md b/README.md index 5cd23cd..c23418b 100644 --- a/README.md +++ b/README.md @@ -2036,7 +2036,9 @@ view would leave a wallet one click from deletion. - **Elements**: - "Back" button, "Delete Wallet" heading - Warning naming the wallet and stating that deletion is permanent and any - funds are unrecoverable without the recovery phrase + funds are unrecoverable without the recovery phrase, the private key or + the extended private key, according to the wallet's kind + (`keyMaterialName()` in `src/popup/views/helpers.js`) - Password input - Error line - "Confirm Delete" button @@ -2077,9 +2079,10 @@ view would leave a wallet one click from deletion. cannot be unlocked again, and that no password is needed to delete it - What deletion does and does not do: it erases the copy of the key stored on this device; nothing on chain changes and no money is moved - - The route back — adding the wallet again with the recovery phrase and a - new password — and, in bold, that without that phrase written down the - deletion loses everything the wallet holds, forever + - The route back — adding the wallet again, with a new password, from a kept + copy of its recovery phrase, private key or extended private key (named as + on DeleteWallet) — and, in bold, that without that copy the deletion loses + everything the wallet holds, forever - That the other wallets are not touched - The wallet's name, and a text input asking for it to be typed back. A name that shows nothing at all (only spaces, or only characters that paint @@ -2137,8 +2140,9 @@ view would leave a wallet one click from deletion. the address **only if it has on-chain activity**. An address that was never used is not found by that scan. The text is written by `recoveryPathText()` rather than sitting in `index.html`, so it can name - the wallet's own kind of key material: an xprv wallet has no recovery - phrase to re-import. + the wallet's own kind of key material through `keyMaterialName()`, as + DeleteWallet and DeleteWalletLostPassword do: an xprv wallet has no + recovery phrase to re-import. - A warning when the address holds anything, ETH or any tracked ERC-20, followed by the holdings themselves via `balanceLinesForAddress()` and the USD total via `formatAddressTotal()` (see diff --git a/src/popup/index.html b/src/popup/index.html index b216be2..ba170ca 100644 --- a/src/popup/index.html +++ b/src/popup/index.html @@ -1123,7 +1123,8 @@

Deleting is permanent. Any - funds will be unrecoverable without your recovery phrase. + funds will be unrecoverable without your + .

@@ -1174,13 +1175,13 @@ money at its addresses is not moved or destroyed.

- If you have the recovery phrase for this wallet written - down, add the wallet again afterwards with a new password - and you will have it back. + If you have kept a copy of this wallet's + , add the + wallet again afterwards with a new password and you will + have it back. If you do not have it written down, deleting this - wallet means losing everything it holds, - forever.If you have not, deleting this wallet means losing + everything it holds, forever.

Your other wallets are not touched.

diff --git a/src/popup/views/deleteAddress.js b/src/popup/views/deleteAddress.js index c8d2a9e..fab26f9 100644 --- a/src/popup/views/deleteAddress.js +++ b/src/popup/views/deleteAddress.js @@ -17,9 +17,9 @@ const { attachCopyHandlers, addressHoldsFunds, balanceLinesForAddress, + keyMaterialName, } = require("./helpers"); const { formatAddressTotal, getAddressValue } = require("../../shared/prices"); -const { walletHasRecoveryPhrase } = require("../../shared/wallet"); const { state, saveState } = require("../../shared/state"); const { canRemoveAddress, @@ -54,9 +54,7 @@ function setFlash(msg) { // The noun follows the wallet: an xprv wallet holds no recovery phrase, and // this screen is offered on xprv wallets too. function recoveryPathText(wallet) { - const secret = walletHasRecoveryPhrase(wallet) - ? "recovery phrase" - : "extended private key"; + const secret = keyMaterialName(wallet); return ( "Getting the address back into this list is not easy, so be sure. " + "Adding an address derives the next unused one, not this one, and " + diff --git a/src/popup/views/deleteWallet.js b/src/popup/views/deleteWallet.js index 5808fcb..384224f 100644 --- a/src/popup/views/deleteWallet.js +++ b/src/popup/views/deleteWallet.js @@ -7,6 +7,7 @@ const { goBack, clearViewStack, onViewLeave, + keyMaterialName, } = require("./helpers"); const { state, saveState } = require("../../shared/state"); const { decryptWithPassword } = require("../../shared/vault"); @@ -75,6 +76,9 @@ function show(walletIdx) { clear(); deleteWalletIndex = walletIdx; $("delete-wallet-name").textContent = displayName(walletIdx); + $("delete-wallet-key-material").textContent = keyMaterialName( + state.wallets[walletIdx], + ); showView("delete-wallet-confirm"); } @@ -94,6 +98,9 @@ function showLostPassword() { clearLostPassword(); $("delete-wallet-lost-name").textContent = name; $("delete-wallet-lost-name-echo").textContent = name; + $("delete-wallet-lost-key-material").textContent = keyMaterialName( + state.wallets[walletIdx], + ); // showView() runs the leave hook of delete-wallet-confirm, which nulls // deleteWalletIndex, so this screen's own selection is recorded after // it and not before. diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index af15ead..7fb5c92 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -25,6 +25,7 @@ const { isDebug } = require("../../shared/log"); const { formatUsd, getPrice } = require("../../shared/prices"); const { state, saveState, currentNetwork } = require("../../shared/state"); const { displaySymbol } = require("../../shared/symbolDisplay"); +const { walletHasRecoveryPhrase } = require("../../shared/wallet"); const { markViewRendered } = require("../viewRouter"); // When views are added, removed, or transitions between them change, @@ -418,6 +419,16 @@ function addressHoldsFunds(addr) { return false; } +// What a wallet was imported from, which is also its only backup: the +// recovery phrase, the private key or the extended private key. The screens +// that tell the user what to keep or import again name it through here, so +// a key or xprv wallet is never sent looking for a recovery phrase. +function keyMaterialName(wallet) { + if (walletHasRecoveryPhrase(wallet)) return "recovery phrase"; + if (wallet.type === "key") return "private key"; + return "extended private key"; +} + // The fewest characters of an address any caller may ask to display. The // 10-character cap inside truncateMiddle() is the other half of the same // guarantee; this is the half that used to be spelled out at each call @@ -725,6 +736,7 @@ module.exports = { balanceLine, balanceLinesForAddress, addressHoldsFunds, + keyMaterialName, unknownableAmount, nativeCurrency, tokenLabel, diff --git a/tests/deleteWalletLostPassword.test.js b/tests/deleteWalletLostPassword.test.js index 0ba4df4..9b5b20c 100644 --- a/tests/deleteWalletLostPassword.test.js +++ b/tests/deleteWalletLostPassword.test.js @@ -259,6 +259,36 @@ describe("reaching the screen", () => { }); }); +// Both screens tell the user what they need to get the wallet back, and a +// key or xprv wallet has no recovery phrase: telling its owner to rely on +// one sends them looking for words that do not exist +// (https://git.eeqj.de/sneak/AutistMask/issues/529). +describe("what the screens say the wallet's backup is", () => { + test.each([ + ["hd", "recovery phrase"], + ["key", "private key"], + ["xprv", "extended private key"], + ])( + "a wallet of type %s is told about its %s", + async (type, keyMaterial) => { + const { deleteWallet, state } = load(); + state.wallets[1].type = type; + + deleteWallet.show(1); + expect(state.currentView).toBe("delete-wallet-confirm"); + expect(node("delete-wallet-key-material").textContent).toBe( + keyMaterial, + ); + + await click("btn-delete-wallet-lost-password"); + expect(state.currentView).toBe(VIEW); + expect(node("delete-wallet-lost-key-material").textContent).toBe( + keyMaterial, + ); + }, + ); +}); + describe("the typed confirmation", () => { test("a name that is not the wallet's deletes nothing", async () => { const { deleteWallet, state, storage } = load();