From 2d28452662242e4fe046fc8bbba18f2178e80069 Mon Sep 17 00:00:00 2001 From: clawbot Date: Wed, 12 Aug 2026 09:12:51 +0000 Subject: [PATCH] feat: remove an address from an HD wallet, behind a confirmation (closes #162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 is not found by that scan. 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. --- README.md | 64 ++++++++- TODO.md | 5 + src/popup/index.html | 56 ++++++++ src/popup/index.js | 6 + src/popup/views/deleteAddress.js | 176 +++++++++++++++++++++++++ src/popup/views/helpers.js | 16 +++ src/popup/views/home.js | 19 ++- src/shared/walletDelete.js | 113 ++++++++++++++-- tests/deleteAddress.test.js | 158 ++++++++++++++++++++++ tests/e2e/run.js | 95 ++++++++++++++ tests/walletDelete.test.js | 216 +++++++++++++++++++++++++++++++ 11 files changed, 905 insertions(+), 19 deletions(-) create mode 100644 src/popup/views/deleteAddress.js create mode 100644 tests/deleteAddress.test.js diff --git a/README.md b/README.md index 73737b0..f94a9de 100644 --- a/README.md +++ b/README.md @@ -138,8 +138,11 @@ transfer, and the recovery phrase screen — which wallet types are offered it, that it holds nothing before the password is accepted, that a wrong password reveals nothing, that leaving it by either route wipes it — including a leave taken while the decrypt is still running — and that reopening the popup does not -land on it. All outbound network is intercepted at the browser level and served -from fixtures in `tests/e2e/network.js`, so the run is deterministic and fully +land on it. It also covers address removal: which wallets offer the control at +all, that the confirmation states the route back rather than showing an empty +paragraph, that leaving the confirmation removes nothing, and that confirming it +does. All outbound network is intercepted at the browser level and served from +fixtures in `tests/e2e/network.js`, so the run is deterministic and fully offline; unrecognised outbound requests are reported as failures rather than silently allowed. @@ -531,8 +534,9 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign. - Wallet list: each wallet shows its name (tap to rename inline) and a "+" button for HD and xprv wallets, then one block per address with "Address N" (bold when active), the ENS name if resolved, the full address, an - `[info]` button, the address USD total, and a balance line for ETH and for - each token shown for that address + `[info]` button, an `[x]` button (only on HD and xprv wallets holding more + than one address), the address USD total, and a balance line for ETH and + for each token shown for that address - "Recent Transactions": up to 25 transactions merged across every address of every wallet, deduplicated by hash and filtered - "Add additional wallet..." link at bottom @@ -542,6 +546,7 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign. - Tap wallet name → inline rename field (no screen change) - "+" on wallet → derives the next address inline (no screen change) - `[info]` on address → **AddressDetail** + - `[x]` on address → **DeleteAddress** - "Send" → **Send** (refuses with a flash message on a zero balance) - "Receive" → **Receive** (shows active address QR) - Tap home tx row → **TransactionDetail** @@ -920,6 +925,55 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign. nothing deleted - "Back" → previous screen (Settings) +#### DeleteAddress (`delete-address-confirm`) + +- **When**: User tapped the `[x]` next to an address on Home. Offered only on HD + and xprv wallets holding more than one address: the last address of a wallet + is never removable, and a key wallet has exactly one. +- **Elements**: + - "Back" button, "Remove Address" heading + - The address's own label ("Address N") and its wallet's name + - The full address (color dot, etherscan link, tap to copy), with the ENS + name above it if resolved + - Explanation that this only stops the wallet tracking the address: nothing + is destroyed, no key is deleted, and funds stay where they are + - The route back, stated with its limit, because the obvious two are both + refused: "+" derives the next unused index (`nextIndex` is a high-water + mark), and re-importing the wallet's key material is rejected as a + duplicate by `findWalletByXpub` while the wallet is still present. What + works is deleting the whole wallet in Settings — password-gated, and it + destroys the stored secret — then importing again, whereupon + `scanForAddresses()` rediscovers 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. + - 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 `getAddressValueUsd()`. The sentence names no figure of its + own: the lines round to four decimals, so a sentence built from a rounded + number would report `0.0000 ETH` for an address holding real money. The + predicate is `addressHoldsFunds()` in `src/popup/views/helpers.js`, + unrounded and token-aware. A balance is a warning, never a refusal. + - The rule that a wallet always keeps at least one address, and that + removing the last one means deleting the wallet from Settings + - Error line + - "Remove Address" button +- **Transitions**: + - "Remove Address" → removes the address and its site permissions, then → + previous screen (Home) with an "Address removed." flash message + - "Back" → previous screen (Home), nothing removed +- **Deliberately not password-gated**, unlike DeleteWallet: a password gates the + disclosure or destruction of a secret, and this does neither. The address + stays derivable from key material the wallet still holds. +- The active address moves only if it was the address removed, and then to the + wallet's first remaining address, with `AUTISTMASK_ACTIVE_CHANGED` broadcast + so a connected site stops being told about an address the user removed + (`src/shared/walletDelete.js`). A selection in any other wallet is left alone; + one in this wallet follows the splice. +- The wallet's derivation counter (`nextIndex`) is not rewound, so "+" derives a + fresh address rather than handing back the one just removed. + #### SettingsAddToken (`settings-addtoken`) - **When**: User tapped "+ Add token" in Settings. Tokens added here are tracked @@ -1403,7 +1457,7 @@ Currently supported: ### Wallet Management - [x] Delete wallet (with confirmation) -- [ ] Delete address from HD wallet (with confirmation) +- [x] Delete address from HD wallet (with confirmation) - [x] Show wallet's recovery phrase (requires password) ### Transactions diff --git a/TODO.md b/TODO.md index a811321..4875d6e 100644 --- a/TODO.md +++ b/TODO.md @@ -44,6 +44,11 @@ undefined identifiers, which is how # Completed Steps +- 2026-08-12: An address can be removed from an HD or xprv wallet behind a + confirmation screen that states nothing is destroyed, sharing the deletion + state transitions with wallet deletion so the selection, site permissions and + active-address broadcast follow the same rules + ([#162](https://git.eeqj.de/sneak/AutistMask/issues/162)). - 2026-08-12: The known-symbol spoof rule moved into `src/shared/symbolSpoof.js` and is now the only copy. The balance list had exempted symbols the token list maps to `null` — `"ETH"` alone — so a fake ETH ERC-20 was hidden from the diff --git a/src/popup/index.html b/src/popup/index.html index 1325c66..3490fd4 100644 --- a/src/popup/index.html +++ b/src/popup/index.html @@ -1142,6 +1142,62 @@ + + +