feat: delete an address from an HD wallet (with confirmation) #162

New Issue

clawbot · 2026-08-09T03:45:57+02:00

clawbot commented

2026-08-09 03:45:57 +02:00

Problem

README.md:879 lists "Delete address from HD wallet (with confirmation)" as an
open TODO, and nothing implements it. Grepping src/ for
deleteAddress / delete-address / removeAddress returns nothing; the only
address-removal code path in the tree is the whole-wallet
addresses.splice at src/popup/views/deleteWallet.js:65.

src/popup/views/home.js:291-310 implements the "+" add-address button, but
there is no corresponding remove control and src/popup/index.html has no
address-delete element. So a user who adds addresses can never tidy them up.

Implementation requirements

Add a per-address remove control for HD wallets only. A key wallet has
exactly one address and no "+" button (README.md:245-250), so it must not
get a remove control either; the same applies to type: "xprv" wallets if
they present a single address.
Confirmation is required. Match the existing delete-wallet treatment
(src/popup/views/deleteWallet.js) for consistency. Decide whether it
should also be password-gated: delete-wallet is, but removing a derived
address destroys no secret — it is recoverable by re-deriving from the same
phrase. Recommend the lighter treatment (an explicit confirm screen, no
password) and state the choice and reasoning in the PR.
The confirmation must make clear this only stops tracking the address
locally; funds at that address are untouched and the address can be
re-derived from the recovery phrase. Full sentences.
Handle the consequences carefully, mirroring the issues found in
delete-wallet (#156):
- If the removed address is the active address, move activeAddress to a
  defined fallback and emit AUTISTMASK_ACTIVE_CHANGED (see
  home.js:277 for the call shape). Consider whether connected dApps need
  broadcastAccountsChanged (src/background/index.js:567-590) — a removed
  address should not remain an authorized account.
- If it is not the active address, leave the user's selection where it is,
  accounting for the index shift from the splice.
- Remove the address's entries from allowedSites and deniedSites, as
  deleteWallet.js:67-71 does.
- Remove any per-address tracked-token state so it does not leak into other
  addresses or reappear.
Decide and document the derivation-index semantics, which is the subtle part:
if a user removes address index 1 of 3, does the next "+" re-derive index 1,
or continue at index 3? Gaps versus renumbering affect
deriveAddressFromXpub and the scanForAddresses gap-limit logic at
src/shared/balances.js:225. Pick one, state it in the PR, and make sure
the "+" button and any address scan agree with it.
Should not be possible to remove the last remaining address of a wallet —
that is what delete-wallet is for. Enforce it and say so in the UI.
Every interactive element needs a visible clickable affordance per the
README policy.

Definition of done

HD wallet address rows expose a remove control; single-address wallets
do not.
Removing an address requires an explicit confirmation that explains the
funds are untouched and the address is re-derivable.
Removing the active address moves the active address to a defined
fallback and broadcasts the change; connected dApps see it.
Removing a non-active address leaves the selection correct after the
index shift.
Site permissions and tracked-token state for the removed address are
cleaned up.
The last address of a wallet cannot be removed.
The derivation-index behaviour after removal is documented in the PR and
is consistent between the "+" button and address scanning.
Unit tests cover the state transitions (active, non-active, last
address).
The README TODO checkbox at README.md:879 is ticked and the Screen Map
updated if a new screen is introduced.
TODO.md updated in the same commit.
make check passes.

## Problem `README.md:879` lists "Delete address from HD wallet (with confirmation)" as an open TODO, and nothing implements it. Grepping `src/` for `deleteAddress` / `delete-address` / `removeAddress` returns nothing; the only address-removal code path in the tree is the whole-wallet `addresses.splice` at `src/popup/views/deleteWallet.js:65`. `src/popup/views/home.js:291-310` implements the "+" add-address button, but there is no corresponding remove control and `src/popup/index.html` has no address-delete element. So a user who adds addresses can never tidy them up. ## Implementation requirements - Add a per-address remove control for **HD wallets only**. A key wallet has exactly one address and no "+" button (`README.md:245-250`), so it must not get a remove control either; the same applies to `type: "xprv"` wallets if they present a single address. - Confirmation is required. Match the existing delete-wallet treatment (`src/popup/views/deleteWallet.js`) for consistency. Decide whether it should also be password-gated: delete-wallet is, but removing a derived address destroys no secret — it is recoverable by re-deriving from the same phrase. Recommend the lighter treatment (an explicit confirm screen, no password) and state the choice and reasoning in the PR. - The confirmation must make clear this only stops tracking the address locally; funds at that address are untouched and the address can be re-derived from the recovery phrase. Full sentences. - Handle the consequences carefully, mirroring the issues found in delete-wallet (#156): - If the removed address is the active address, move `activeAddress` to a defined fallback and emit `AUTISTMASK_ACTIVE_CHANGED` (see `home.js:277` for the call shape). Consider whether connected dApps need `broadcastAccountsChanged` (`src/background/index.js:567-590`) — a removed address should not remain an authorized account. - If it is not the active address, leave the user's selection where it is, accounting for the index shift from the splice. - Remove the address's entries from `allowedSites` and `deniedSites`, as `deleteWallet.js:67-71` does. - Remove any per-address tracked-token state so it does not leak into other addresses or reappear. - Decide and document the derivation-index semantics, which is the subtle part: if a user removes address index 1 of 3, does the next "+" re-derive index 1, or continue at index 3? Gaps versus renumbering affect `deriveAddressFromXpub` and the `scanForAddresses` gap-limit logic at `src/shared/balances.js:225`. Pick one, state it in the PR, and make sure the "+" button and any address scan agree with it. - Should not be possible to remove the last remaining address of a wallet — that is what delete-wallet is for. Enforce it and say so in the UI. - Every interactive element needs a visible clickable affordance per the README policy. ## Definition of done - [ ] HD wallet address rows expose a remove control; single-address wallets do not. - [ ] Removing an address requires an explicit confirmation that explains the funds are untouched and the address is re-derivable. - [ ] Removing the active address moves the active address to a defined fallback and broadcasts the change; connected dApps see it. - [ ] Removing a non-active address leaves the selection correct after the index shift. - [ ] Site permissions and tracked-token state for the removed address are cleaned up. - [ ] The last address of a wallet cannot be removed. - [ ] The derivation-index behaviour after removal is documented in the PR and is consistent between the "+" button and address scanning. - [ ] Unit tests cover the state transitions (active, non-active, last address). - [ ] The README TODO checkbox at `README.md:879` is ticked and the Screen Map updated if a new screen is introduced. - [ ] `TODO.md` updated in the same commit. - [ ] `make check` passes.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:45:57 +02:00

clawbot referenced this issue

2026-08-09 03:46:51 +02:00

docs: README Screen Map omits three shipped screens and misdescribes four flows #164

clawbot referenced this issue

2026-08-09 04:54:11 +02:00

decision: no browser in the agent environment, so several 1.0.0 items cannot have their DoD verified #173

clawbot referenced this issue

2026-08-09 16:09:35 +02:00

decision: no browser in the agent environment, so several 1.0.0 items cannot have their DoD verified #173

clawbot referenced this issue

2026-08-11 14:23:34 +02:00