feat: remove an address from an HD wallet, behind a confirmation (closes #162)
All checks were successful
check / check (push) Successful in 24s
All checks were successful
check / check (push) Successful in 24s
Address rows on Home now carry an [x] control on wallets that derive their addresses from an extended key and hold more than one; it opens a confirmation screen before anything is removed. Removing an address destroys nothing: it stays derivable from key material the wallet still holds, and any funds at it stay where they are. That is also why the screen is not password-gated, unlike delete-wallet — a password gates the disclosure or destruction of a secret, and this does neither. Getting the address back into the list is another matter, and the copy states it exactly rather than promising a route the app refuses. "+" derives the next unused index, because the derivation counter is a high-water mark and is not rewound, and re-importing the wallet's key material is rejected as a duplicate for as long as the wallet is present — which it always is here, since a wallet never gives up its last address. What works is deleting the whole wallet in Settings, which asks for the password and destroys the stored secret, then importing again: the scan that follows rediscovers the address only if it has on-chain activity, and an address that was never used does not come back at all. The text is built by recoveryPathText() rather than sitting in index.html so it can name the wallet's own kind of key material, an xprv wallet having no recovery phrase to re-import. A balance is surfaced as a warning, never a refusal, and holding something means any ERC-20 as well as ETH, at any size: an address with no ETH and a stablecoin position must not get a blank line on the screen whose job is to warn. The warning names no figure of its own, because the balance lines round to four decimals and a sentence built from a rounded number would report 0.0000 ETH for an address holding real money; the amounts come from the same balanceLinesForAddress() and getAddressValueUsd() every other screen uses. The state transition lives next to the wallet one in src/shared/walletDelete.js and shares its address comparison, site-permission cleanup and broadcast, so the rules match one level down: the last address of a wallet is never removable, the selection moves only when it was the address removed, an index after the splice is decremented, a selection in another wallet is untouched, and AUTISTMASK_ACTIVE_CHANGED is broadcast when the active address moves so a connected site stops being told about an address the user removed.
This commit is contained in:
158
tests/deleteAddress.test.js
Normal file
158
tests/deleteAddress.test.js
Normal file
@@ -0,0 +1,158 @@
|
||||
// Tests for the copy on the address-removal confirmation (issue #162).
|
||||
//
|
||||
// The screen's whole job is to warn before a destructive-looking action, so
|
||||
// the copy is the substance and is tested as such. Two things it must not
|
||||
// get wrong: what it takes to get the address back — the app refuses both
|
||||
// obvious routes — and what counts as holding something, which is any
|
||||
// ERC-20 as well as ETH, at any size, including a balance that rounds to
|
||||
// zero at the four decimals the balance lines render. The DOM behaviour
|
||||
// around them is driven against the real popup by tests/e2e/run.js.
|
||||
|
||||
// helpers.js pulls in state.js, which reads chrome.storage.local at load.
|
||||
globalThis.chrome = {
|
||||
storage: { local: { get: async () => ({}), set: async () => {} } },
|
||||
};
|
||||
|
||||
const { addressHoldsFunds } = require("../src/popup/views/helpers");
|
||||
const {
|
||||
recoveryPathText,
|
||||
balanceWarningHtml,
|
||||
} = require("../src/popup/views/deleteAddress");
|
||||
const { prices, clearPrices } = require("../src/shared/prices");
|
||||
|
||||
const USDC = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
|
||||
|
||||
const EMPTY = { address: "0x1", balance: "0.0000", tokenBalances: [] };
|
||||
const ETH_ONLY = { address: "0x1", balance: "1.5", tokenBalances: [] };
|
||||
const DUST = { address: "0x1", balance: "0.00001", tokenBalances: [] };
|
||||
const TOKEN_ONLY = {
|
||||
address: "0x1",
|
||||
balance: "0.0000",
|
||||
tokenBalances: [{ address: USDC, symbol: "USDC", balance: "2500.0" }],
|
||||
};
|
||||
const ZERO_TOKEN = {
|
||||
address: "0x1",
|
||||
balance: "0",
|
||||
tokenBalances: [{ address: USDC, symbol: "USDC", balance: "0" }],
|
||||
};
|
||||
|
||||
afterEach(() => {
|
||||
clearPrices();
|
||||
});
|
||||
|
||||
describe("what the screen says it takes to get the address back", () => {
|
||||
// The screen used to promise the address "can be brought back at any
|
||||
// time by importing this wallet's recovery phrase again". That import is
|
||||
// refused as a duplicate for as long as the wallet is present, which it
|
||||
// always is here — a wallet never gives up its last address.
|
||||
test("it does not promise a re-import while the wallet is here", () => {
|
||||
const text = recoveryPathText({ type: "hd" });
|
||||
expect(text).not.toMatch(/at any time/);
|
||||
expect(text).toContain("is refused while this wallet is still here");
|
||||
});
|
||||
|
||||
test("it names deleting the whole wallet as the route back", () => {
|
||||
expect(recoveryPathText({ type: "hd" })).toContain(
|
||||
"delete the whole wallet in Settings",
|
||||
);
|
||||
});
|
||||
|
||||
// The scan after a re-import finds used addresses only, so an address
|
||||
// that never saw a transaction does not come back at all. Saying so is
|
||||
// the difference between a warning and a false reassurance.
|
||||
test("it states the limit: only on-chain activity is found", () => {
|
||||
const text = recoveryPathText({ type: "hd" });
|
||||
expect(text).toContain("only finds addresses that have on-chain");
|
||||
expect(text).toContain("never been used is not found by it");
|
||||
});
|
||||
|
||||
// The screen is offered on xprv wallets too, and an xprv wallet holds no
|
||||
// recovery phrase — telling its owner to import one would send them
|
||||
// looking for words that do not exist.
|
||||
test("an xprv wallet is told about its extended private key", () => {
|
||||
const text = recoveryPathText({ type: "xprv" });
|
||||
expect(text).toContain("extended private key");
|
||||
expect(text).not.toContain("recovery phrase");
|
||||
});
|
||||
|
||||
test("an HD wallet is told about its recovery phrase", () => {
|
||||
const text = recoveryPathText({ type: "hd" });
|
||||
expect(text).toContain("recovery phrase");
|
||||
expect(text).not.toContain("extended private key");
|
||||
});
|
||||
});
|
||||
|
||||
describe("whether an address holds anything", () => {
|
||||
test("ETH counts", () => {
|
||||
expect(addressHoldsFunds(ETH_ONLY)).toBe(true);
|
||||
});
|
||||
|
||||
// The case that decides the screen: no ETH at all, and $2500 of a
|
||||
// stablecoin sitting at the address.
|
||||
test("an ERC-20 balance counts even with no ETH", () => {
|
||||
expect(addressHoldsFunds(TOKEN_ONLY)).toBe(true);
|
||||
});
|
||||
|
||||
// 0.00001 ETH renders as "0.0000" at four decimals. It is still money.
|
||||
test("an ETH balance below the displayed precision counts", () => {
|
||||
expect(addressHoldsFunds(DUST)).toBe(true);
|
||||
});
|
||||
|
||||
test("an address holding nothing does not", () => {
|
||||
expect(addressHoldsFunds(EMPTY)).toBe(false);
|
||||
expect(addressHoldsFunds(ZERO_TOKEN)).toBe(false);
|
||||
});
|
||||
|
||||
test("a missing address or missing fields do not", () => {
|
||||
expect(addressHoldsFunds(undefined)).toBe(false);
|
||||
expect(addressHoldsFunds({ address: "0x1" })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("the balance warning on the removal confirmation", () => {
|
||||
test("an address holding nothing gets a blank line, not a warning", () => {
|
||||
expect(balanceWarningHtml(EMPTY)).toBe(" ");
|
||||
expect(balanceWarningHtml(ZERO_TOKEN)).toBe(" ");
|
||||
});
|
||||
|
||||
test("an ERC-20-only address is warned about, and its token listed", () => {
|
||||
const html = balanceWarningHtml(TOKEN_ONLY);
|
||||
expect(html).toContain("This address holds a balance.");
|
||||
expect(html).toContain("does not move or spend anything");
|
||||
expect(html).toContain("USDC");
|
||||
expect(html).toContain("2500.0000");
|
||||
});
|
||||
|
||||
// The rendered line says 0.0000 for this address — that is the display
|
||||
// format, shared with Home and AddressDetail — and the warning is shown
|
||||
// all the same, because the balance is not zero.
|
||||
test("an ETH balance that renders as 0.0000 is warned about", () => {
|
||||
const html = balanceWarningHtml(DUST);
|
||||
expect(html).toContain("This address holds a balance.");
|
||||
expect(html).toContain("<span>0.0000</span>");
|
||||
});
|
||||
|
||||
// The sentence must not assert an amount, because any amount it could
|
||||
// assert has been rounded: "This address holds 0.0000 ETH." is what the
|
||||
// rounded form produces for an address that holds real money.
|
||||
test("the warning sentence asserts no rounded amount", () => {
|
||||
for (const addr of [DUST, ETH_ONLY, TOKEN_ONLY]) {
|
||||
expect(balanceWarningHtml(addr)).not.toMatch(
|
||||
/holds [\d.]+ (ETH|USDC)/,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test("the USD total is shown when prices are known", () => {
|
||||
prices.ETH = 2000;
|
||||
prices.USDC = 1;
|
||||
expect(balanceWarningHtml(TOKEN_ONLY)).toContain("Total: $2,500.00");
|
||||
expect(balanceWarningHtml(ETH_ONLY)).toContain("Total: $3,000.00");
|
||||
});
|
||||
|
||||
// getAddressValueUsd() returns null on testnet and before the first
|
||||
// price fetch. A "Total: $0.00" there would be a lie about the holdings.
|
||||
test("no USD total is shown when prices are not known", () => {
|
||||
expect(balanceWarningHtml(TOKEN_ONLY)).not.toContain("Total:");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user