Address rows on Home now carry an [x] control, on wallets that derive their
addresses from an extended key (hd, xprv) and hold more than one address.
It opens a new confirmation screen, DeleteAddress (delete-address-confirm),
which removes the address only when the user confirms it there.
What deletion means for an HD address
These addresses are derived from key material the wallet still holds, so
"delete" destroys no key and moves no funds. The screen says that, and answers
the three questions as follows.
Does deleting address N of M renumber the rest? The derivation indices do
not renumber. wallet.nextIndex is a high-water mark and is deliberately not
rewound, so removing index 1 of 3 leaves a gap and the next + derives index 3,
not index 1. That was chosen over renumbering for two reasons: the address
records do not store their derivation index (only their position in wallet.addresses), so renumbering would need a schema change; and a + that
re-derived the index just removed would silently resurrect the address the user
had just asked to be rid of. The gap is within what scanForAddresses()
(src/shared/balances.js) tolerates — its gap limit is 5 and it extends the
scan past every used index it finds — so a later import still discovers a
skipped-but-used index.
One consequence is worth stating plainly: the display labels are positional
(Address ${ai + 1} in src/popup/views/home.js), so what was "Address 3"
shows as "Address 2" after "Address 2" is removed, even though its derivation
index is unchanged. That is pre-existing behaviour of the label, not something
introduced here, and correcting it would mean storing the derivation index on
each address record. Flagged rather than changed, since it is outside this
issue.
Can it be brought back? Not easily, and the screen says so rather than
promising otherwise. Both obvious routes are refused by the app:
+ derives the next unused index, per the high-water mark above.
Re-importing the wallet's key material is rejected as a duplicate by findWalletByXpub() in src/popup/views/addWallet.js for as long as the
wallet is present — which it always is on this screen, since canRemoveAddress
guarantees the wallet keeps at least one address.
What works is deleting the whole wallet from Settings — password-gated, and
it destroys the stored encrypted secret — then importing again, after which scanForAddresses() rediscovers the address only if it has on-chain
activity. An address that was never used does not come back at all, and the
copy says that too. 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 — which this screen is also offered on — holds no recovery
phrase to re-import.
What happens if the address holds a balance? Allowed, with a warning, not
blocked. Blocking would be wrong: the funds are at the address on-chain, not in
this list, and they stay there either way — refusing would only strand the user
with a row they cannot tidy up.
"Holds" means ETH or any ERC-20, at any size: addressHoldsFunds() in src/popup/views/helpers.js, unrounded and token-aware. The warning sentence
names no figure of its own, because the balance lines round to four decimals and
a sentence built from a rounded number reports "0.0000 ETH" for an address
holding real money. The amounts come from the same balanceLinesForAddress()
and getAddressValueUsd() every other screen uses; the USD total is omitted
rather than printed as $0.00 when prices are unknown (testnet, or before the
first fetch).
Not password-gated
Unlike DeleteWallet, no password is asked for. A password gates the disclosure
or destruction of a secret, and this does neither. An explicit confirmation
screen is the proportionate treatment, as the issue recommends.
Shared logic, not a second implementation
The state transition lives next to the wallet one in src/shared/walletDelete.js and shares its address comparison
(sameAddress, case-insensitive), site-permission cleanup
(dropSitePermissions) and broadcastActiveChanged. removeWalletFromState
was refactored onto those helpers in the same commit; its behaviour is
unchanged and its existing tests still pass untouched.
The rules match the wallet-level ones, one level down:
The last address of a wallet is never removable — that is what delete-wallet
is for. Enforced in canRemoveAddress(), which is the same predicate the
render uses to decide whether to draw the control, so the gate cannot drift
between the UI and the transition.
hasWallet and the wallet list are untouched: the wallet keeps at least one
address, so neither can change.
The selection moves only when the removed address was the selected one,
and then to that wallet's first remaining address. A selection at a later
index in the same wallet is decremented to follow the splice; a selection in
any other wallet, and selectedWallet itself, are left alone.
activeAddress moves only when it was the removed address, and AUTISTMASK_ACTIVE_CHANGED is then sent on the same path every other
selection change uses, so the background re-emits accountsChanged and a
connected dApp stops reporting an address the user removed. State is saved
before the broadcast, because the background reads the active address back
out of storage.
Site permissions (allowedSites, deniedSites) are dropped for the removed
address only.
Per-address token state needs no separate cleanup: state.trackedTokens is
global to the profile, and the per-address balances live on the address record
(addr.tokenBalances), which goes with it in the splice.
Scope
The diff stays out of the private-key export path
(#221): the control is on
Home, not AddressDetail, and the changes in src/popup/views/helpers.js are one
entry in the VIEWS array and the new addressHoldsFunds() predicate, both
away from the view-leave machinery.
Test Suites: 20 passed, 20 total
Tests: 447 passed, 447 total
All matched files use Prettier code style!
Run again in the pinned container via script/cibuild, where the layer is shown
executing rather than CACHED:
#14 [7/8] RUN make check
#14 9.923 Test Suites: 20 passed, 20 total
#14 9.923 Tests: 447 passed, 447 total
#14 17.46 All matched files use Prettier code style!
make test-e2e green, 17/17, driving the real popup in the pinned container:
ok 15 - only a wallet that can spare an address offers to remove one (#162)
ok 16 - leaving the removal confirmation removes nothing (#162)
ok 17 - confirming removes the address and returns Home (#162)
# 17/17 tests passed
Test 16 is the confirmation gate: it opens the confirmation, asserts the screen
actually states the route back (an empty paragraph would mean the user is
confirming with no idea what it takes to undo), leaves by "Back" — which
re-renders Home, so the count after it is a real measurement rather than a stale
screen — and asserts the address is still there.
Unit coverage
tests/walletDelete.test.js, sixteen new cases for the state transition:
deleting a non-selected address — selection intact, index arithmetic correct
after the splice (a selection after the removed index is decremented, one
before it is not, one in another wallet is untouched)
deleting the selected/active address — selection and active address move to
the wallet's first address, activeAddressChanged reported so the broadcast
fires
the active address matched case-insensitively
deleting the only address of a wallet — refused, nothing mutated; and the
same refusal reached by wearing an HD wallet down to one address
the confirmation gate's predicate: canRemoveAddress for hd/xprv with more
than one address, and against a single-address wallet, a key wallet, and a
missing or typeless wallet
site permissions dropped for the removed address only
the derivation counter not rewound
out-of-range wallet and address indices refused
tests/deleteAddress.test.js, sixteen cases for the copy, which is the
substance of a confirmation screen and is tested as such: that it does not
promise a re-import while the wallet is present, that it names deleting the
whole wallet as the route back, that it states the on-chain-activity limit,
and that an xprv wallet is told about its extended private key rather than a
recovery phrase; then that an ERC-20-only address and a sub-0.0001 ETH balance
both raise the warning, that an address holding nothing does not, that the
sentence asserts no rounded amount, and that the USD total appears only when
prices are known.
Each new guard was mutation-checked and killed only the tests claiming to cover
it: a token-blind addressHoldsFunds failed the three ERC-20 cases and nothing
else; comparing the four-decimal rounded balance instead of the raw one failed
the two dust cases and nothing else; a wallet-blind noun in recoveryPathText
failed the xprv case and nothing else.
Docs
README.md: the Screen Map gains a DeleteAddress entry in the established
format — including the route back with its limit, and why the balance warning
is token-aware and names no figure — Home's element list and transitions gain
the [x] control, the End-to-End Tests section names the new coverage, and the
"Delete address from HD wallet (with confirmation)" TODO checkbox is ticked. TODO.md gains one Completed Steps line.
Closes [#162](https://git.eeqj.de/sneak/AutistMask/issues/162).
## What this adds
Address rows on Home now carry an `[x]` control, on wallets that derive their
addresses from an extended key (`hd`, `xprv`) and hold more than one address.
It opens a new confirmation screen, **DeleteAddress** (`delete-address-confirm`),
which removes the address only when the user confirms it there.
## What deletion means for an HD address
These addresses are derived from key material the wallet still holds, so
"delete" destroys no key and moves no funds. The screen says that, and answers
the three questions as follows.
**Does deleting address N of M renumber the rest?** The derivation indices do
not renumber. `wallet.nextIndex` is a high-water mark and is deliberately not
rewound, so removing index 1 of 3 leaves a gap and the next `+` derives index 3,
not index 1. That was chosen over renumbering for two reasons: the address
records do not store their derivation index (only their position in
`wallet.addresses`), so renumbering would need a schema change; and a `+` that
re-derived the index just removed would silently resurrect the address the user
had just asked to be rid of. The gap is within what `scanForAddresses()`
(`src/shared/balances.js`) tolerates — its gap limit is 5 and it extends the
scan past every used index it finds — so a later import still discovers a
skipped-but-used index.
One consequence is worth stating plainly: the **display labels** are positional
(`Address ${ai + 1}` in `src/popup/views/home.js`), so what was "Address 3"
shows as "Address 2" after "Address 2" is removed, even though its derivation
index is unchanged. That is pre-existing behaviour of the label, not something
introduced here, and correcting it would mean storing the derivation index on
each address record. Flagged rather than changed, since it is outside this
issue.
**Can it be brought back?** Not easily, and the screen says so rather than
promising otherwise. Both obvious routes are refused by the app:
- `+` derives the next unused index, per the high-water mark above.
- Re-importing the wallet's key material is rejected as a duplicate by
`findWalletByXpub()` in `src/popup/views/addWallet.js` for as long as the
wallet is present — which it always is on this screen, since `canRemoveAddress`
guarantees the wallet keeps at least one address.
What works is deleting the **whole wallet** from Settings — password-gated, and
it destroys the stored encrypted secret — then importing again, after which
`scanForAddresses()` rediscovers the address **only if it has on-chain
activity**. An address that was never used does not come back at all, and the
copy says that too. 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 — which this screen is also offered on — holds no recovery
phrase to re-import.
**What happens if the address holds a balance?** Allowed, with a warning, not
blocked. Blocking would be wrong: the funds are at the address on-chain, not in
this list, and they stay there either way — refusing would only strand the user
with a row they cannot tidy up.
"Holds" means ETH **or any ERC-20**, at any size: `addressHoldsFunds()` in
`src/popup/views/helpers.js`, unrounded and token-aware. The warning sentence
names no figure of its own, because the balance lines round to four decimals and
a sentence built from a rounded number reports "0.0000 ETH" for an address
holding real money. The amounts come from the same `balanceLinesForAddress()`
and `getAddressValueUsd()` every other screen uses; the USD total is omitted
rather than printed as `$0.00` when prices are unknown (testnet, or before the
first fetch).
## Not password-gated
Unlike DeleteWallet, no password is asked for. A password gates the disclosure
or destruction of a secret, and this does neither. An explicit confirmation
screen is the proportionate treatment, as the issue recommends.
## Shared logic, not a second implementation
The state transition lives next to the wallet one in
`src/shared/walletDelete.js` and shares its address comparison
(`sameAddress`, case-insensitive), site-permission cleanup
(`dropSitePermissions`) and `broadcastActiveChanged`. `removeWalletFromState`
was refactored onto those helpers in the same commit; its behaviour is
unchanged and its existing tests still pass untouched.
The rules match the wallet-level ones, one level down:
- The last address of a wallet is never removable — that is what delete-wallet
is for. Enforced in `canRemoveAddress()`, which is the same predicate the
render uses to decide whether to draw the control, so the gate cannot drift
between the UI and the transition.
- `hasWallet` and the wallet list are untouched: the wallet keeps at least one
address, so neither can change.
- The selection moves **only** when the removed address was the selected one,
and then to that wallet's first remaining address. A selection at a later
index in the same wallet is decremented to follow the splice; a selection in
any other wallet, and `selectedWallet` itself, are left alone.
- `activeAddress` moves only when it was the removed address, and
`AUTISTMASK_ACTIVE_CHANGED` is then sent on the same path every other
selection change uses, so the background re-emits `accountsChanged` and a
connected dApp stops reporting an address the user removed. State is saved
before the broadcast, because the background reads the active address back
out of storage.
- Site permissions (`allowedSites`, `deniedSites`) are dropped for the removed
address only.
Per-address token state needs no separate cleanup: `state.trackedTokens` is
global to the profile, and the per-address balances live on the address record
(`addr.tokenBalances`), which goes with it in the splice.
## Scope
The diff stays out of the private-key export path
([#221](https://git.eeqj.de/sneak/AutistMask/issues/221)): the control is on
Home, not AddressDetail, and the changes in `src/popup/views/helpers.js` are one
entry in the `VIEWS` array and the new `addressHoldsFunds()` predicate, both
away from the view-leave machinery.
## Verification
`make check` green, rebased onto `next` at
[`bd4bdca`](https://git.eeqj.de/sneak/AutistMask/commit/bd4bdca):
```
Test Suites: 20 passed, 20 total
Tests: 447 passed, 447 total
All matched files use Prettier code style!
```
Run again in the pinned container via `script/cibuild`, where the layer is shown
executing rather than `CACHED`:
```
#14 [7/8] RUN make check
#14 9.923 Test Suites: 20 passed, 20 total
#14 9.923 Tests: 447 passed, 447 total
#14 17.46 All matched files use Prettier code style!
```
`make test-e2e` green, 17/17, driving the real popup in the pinned container:
```
ok 15 - only a wallet that can spare an address offers to remove one (#162)
ok 16 - leaving the removal confirmation removes nothing (#162)
ok 17 - confirming removes the address and returns Home (#162)
# 17/17 tests passed
```
Test 16 is the confirmation gate: it opens the confirmation, asserts the screen
actually states the route back (an empty paragraph would mean the user is
confirming with no idea what it takes to undo), leaves by "Back" — which
re-renders Home, so the count after it is a real measurement rather than a stale
screen — and asserts the address is still there.
### Unit coverage
`tests/walletDelete.test.js`, sixteen new cases for the state transition:
- deleting a non-selected address — selection intact, index arithmetic correct
after the splice (a selection after the removed index is decremented, one
before it is not, one in another wallet is untouched)
- deleting the selected/active address — selection and active address move to
the wallet's first address, `activeAddressChanged` reported so the broadcast
fires
- the active address matched case-insensitively
- deleting the only address of a wallet — refused, nothing mutated; and the
same refusal reached by wearing an HD wallet down to one address
- the confirmation gate's predicate: `canRemoveAddress` for hd/xprv with more
than one address, and against a single-address wallet, a key wallet, and a
missing or typeless wallet
- site permissions dropped for the removed address only
- the derivation counter not rewound
- out-of-range wallet and address indices refused
`tests/deleteAddress.test.js`, sixteen cases for the copy, which is the
substance of a confirmation screen and is tested as such: that it does not
promise a re-import while the wallet is present, that it names deleting the
whole wallet as the route back, that it states the on-chain-activity limit,
and that an `xprv` wallet is told about its extended private key rather than a
recovery phrase; then that an ERC-20-only address and a sub-0.0001 ETH balance
both raise the warning, that an address holding nothing does not, that the
sentence asserts no rounded amount, and that the USD total appears only when
prices are known.
Each new guard was mutation-checked and killed only the tests claiming to cover
it: a token-blind `addressHoldsFunds` failed the three ERC-20 cases and nothing
else; comparing the four-decimal rounded balance instead of the raw one failed
the two dust cases and nothing else; a wallet-blind noun in `recoveryPathText`
failed the `xprv` case and nothing else.
## Docs
`README.md`: the Screen Map gains a **DeleteAddress** entry in the established
format — including the route back with its limit, and why the balance warning
is token-aware and names no figure — Home's element list and transitions gain
the `[x]` control, the End-to-End Tests section names the new coverage, and the
"Delete address from HD wallet (with confirmation)" TODO checkbox is ticked.
`TODO.md` gains one Completed Steps line.
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, and the copy says so: the address stays
derivable from key material the wallet still holds, any funds at it stay
where they are, and importing the recovery phrase brings it back. 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. A
balance is surfaced as a warning line, never as a refusal.
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.
The wallet's derivation counter is a high-water mark and is not rewound, so
"+" derives a fresh index rather than handing back the address just removed.
clawbot
self-assigned this 2026-08-11 15:42:03 +02:00
Head reviewed: 125566d. Evidence from my own clone, not tracker CI (see #220): make check green — 13 suites / 341 tests, 13.8s wall, no cached-result markers; prettier clean. make test-e2e green — 16/16 in the pinned container, tests 14-16 new. Six mutations of the new guards (last-address length > 1, the hd/xprv type gate, the selectedAddress decrement, the activeWasRemoved fallback, dropSitePermissions, sameAddress case-folding) each killed exactly the tests that claim to cover them; no vacuous assertions found.
1. The confirmation promises a recovery path the app refuses
src/popup/index.html:1133-1138 (and README.md:888, and the PR body) tell the user the address "can be brought back at any time by importing this wallet's recovery phrase again".
That import is hard-refused. src/popup/views/addWallet.js:120-126:
constxpubDup=findWalletByXpub(xpub);if(xpubDup){showFlash("This recovery phrase is already added ("+xpubDup.name+").");return;}
While the wallet exists — which it always does at this point, since canRemoveAddress() guarantees the wallet keeps at least one address — re-importing the phrase is rejected outright. + will not bring it back either, by this PR's own deliberate nextIndex high-water-mark design. So the only route back is: delete the whole wallet from Settings (password-gated, destroys the stored encrypted phrase), then import the phrase again and let scanForAddresses() rediscover it — and that rescan only restores addresses with on-chain activity.
Reproduction: HD wallet, + to three addresses, remove Address 2, then Add wallet -> Import recovery phrase, enter the same phrase. Flash: "This recovery phrase is already added (Wallet 1)."
Why it matters: this is the load-bearing assurance on a destructive confirmation screen in a wallet. The user is told the action is trivially reversible; it is not, and the stated undo is one the app rejects. The technical claim in the PR body ("It comes back by importing the wallet's recovery phrase again") is likewise unqualified.
Acceptable: state the path that actually exists and its limit, e.g. "...brought back by deleting this wallet from Settings and importing its recovery phrase again, which rediscovers every address that has been used." Mirror the wording in README.md:888 and the PR body. Alternatively, provide a real in-app re-add path for a gapped index — but the copy fix is the minimum.
2. The balance warning is ETH-only and prints a rounded zero
Two problems on the screen whose stated purpose is to warn:
An address holding no ETH but ERC-20 tokens (addr.tokenBalances, which the Home row already prices via getAddressValueUsd() in src/popup/views/helpers.js) falls into the   branch and shows no warning at all. Reproduction: address with 0 ETH and any tracked token balance -> blank warning line.
toFixed(4) on a small nonzero balance renders "This address holds 0.0000 ETH." Reproduction: addr.balance = "0.00005" -> the sentence asserts a holding of 0.0000 ETH.
Acceptable: warn on the address's total value, not its ETH alone — reuse getAddressValueUsd() and/or balanceLinesForAddress() rather than a second, narrower notion of "holds" — and never print a rounded-to-zero figure as the quantity held.
3. Commit identity does not match the repo
125566d is authored and committed by clawbot <clawbot@eeqj.de>. Every commit on next uses clawbot <clawbot@noreply.example.org> (12/12 checked). Acceptable: re-author the commit with the noreply.example.org identity for author and committer.
4. Not fast-forwardable onto current next
Branch is based on 6f6bc2e; origin/next is now 158278d. No textual conflicts (git merge-tree clean, Gitea reports mergeable), but the branch will not fast-forward. Rebase onto current origin/next and re-run make check and make test-e2e.
Verified and passing
Definition of done in #162 is otherwise met: control offered only on hd/xprv with >1 address; last address unremovable, enforced by the same predicate the render uses; selection index arithmetic; activeAddress fallback plus AUTISTMASK_ACTIVE_CHANGED (background at src/background/index.js:867 re-emits accountsChanged, and only the active address is ever exposed to a site, so a non-active removal correctly needs no broadcast); allowedSites/deniedSites dropped for the removed address only; saveState() is a single whole-object storage.set, so a crash mid-delete cannot half-apply it; save precedes broadcast; double-fire is impossible (target is nulled synchronously before the first await); delete-address-confirm is correctly absent from RESTORABLE_VIEWS; no key material touched. Derivation semantics documented and consistent: + derives at wallet.nextIndex and never rewinds, and scanForAddresses() (gap limit 5, extended past each used index) tolerates the gap. Single commit, title carries (closes #162), one TODO.md bullet, no attribution trailers, no competitor named, "recovery phrase"/"address"/"password" terminology correct, error strings are full sentences, make fmt clean, no scope creep into the private-key export path.
Disclosures
This repo's script/lint runs prettier --check . — the same command as script/fmt-check; there is no eslint. My lint evidence is therefore formatting only, and no host/container lint discrepancy is possible here. Not a defect in this PR.
Re-derivation and gap behaviour were verified by reading scanForAddresses() and the + handler, not by running a chain scan against a live RPC.
connectedSites (the background's in-memory origin + ":" + address map) is not cleared for a removed address. removeWalletFromState() has never cleared it either, so this is pre-existing and consistent rather than introduced here; not filed as a defect.
No password gate: deliberate, matches the recommendation in #162, and the reasoning given is sound.
## Review: FAIL (`needs-rework`)
Head reviewed: `125566d`. Evidence from my own clone, not tracker CI (see [#220](https://git.eeqj.de/sneak/AutistMask/issues/220)): `make check` green — 13 suites / 341 tests, 13.8s wall, no cached-result markers; prettier clean. `make test-e2e` green — 16/16 in the pinned container, tests 14-16 new. Six mutations of the new guards (last-address `length > 1`, the hd/xprv type gate, the `selectedAddress` decrement, the `activeWasRemoved` fallback, `dropSitePermissions`, `sameAddress` case-folding) each killed exactly the tests that claim to cover them; no vacuous assertions found.
### 1. The confirmation promises a recovery path the app refuses
`src/popup/index.html:1133-1138` (and `README.md:888`, and the PR body) tell the user the address "can be brought back at any time by importing this wallet's recovery phrase again".
That import is hard-refused. `src/popup/views/addWallet.js:120-126`:
```js
const xpubDup = findWalletByXpub(xpub);
if (xpubDup) {
showFlash("This recovery phrase is already added (" + xpubDup.name + ").");
return;
}
```
While the wallet exists — which it always does at this point, since `canRemoveAddress()` guarantees the wallet keeps at least one address — re-importing the phrase is rejected outright. `+` will not bring it back either, by this PR's own deliberate `nextIndex` high-water-mark design. So the only route back is: delete the whole wallet from Settings (password-gated, destroys the stored encrypted phrase), then import the phrase again and let `scanForAddresses()` rediscover it — and that rescan only restores addresses with on-chain activity.
Reproduction: HD wallet, `+` to three addresses, remove Address 2, then Add wallet -> Import recovery phrase, enter the same phrase. Flash: "This recovery phrase is already added (Wallet 1)."
Why it matters: this is the load-bearing assurance on a destructive confirmation screen in a wallet. The user is told the action is trivially reversible; it is not, and the stated undo is one the app rejects. The technical claim in the PR body ("It comes back by importing the wallet's recovery phrase again") is likewise unqualified.
Acceptable: state the path that actually exists and its limit, e.g. "...brought back by deleting this wallet from Settings and importing its recovery phrase again, which rediscovers every address that has been used." Mirror the wording in `README.md:888` and the PR body. Alternatively, provide a real in-app re-add path for a gapped index — but the copy fix is the minimum.
### 2. The balance warning is ETH-only and prints a rounded zero
`src/popup/views/deleteAddress.js:52-61`:
```js
const balance = parseFloat(addr.balance || "0");
$("delete-address-balance").innerHTML =
balance > 0 ? "This address holds " + balance.toFixed(4) + " ETH. ..." : "&nbsp;";
```
Two problems on the screen whose stated purpose is to warn:
- An address holding no ETH but ERC-20 tokens (`addr.tokenBalances`, which the Home row already prices via `getAddressValueUsd()` in `src/popup/views/helpers.js`) falls into the `&nbsp;` branch and shows no warning at all. Reproduction: address with 0 ETH and any tracked token balance -> blank warning line.
- `toFixed(4)` on a small nonzero balance renders "This address holds 0.0000 ETH." Reproduction: `addr.balance = "0.00005"` -> the sentence asserts a holding of 0.0000 ETH.
Acceptable: warn on the address's total value, not its ETH alone — reuse `getAddressValueUsd()` and/or `balanceLinesForAddress()` rather than a second, narrower notion of "holds" — and never print a rounded-to-zero figure as the quantity held.
### 3. Commit identity does not match the repo
`125566d` is authored and committed by `clawbot <clawbot@eeqj.de>`. Every commit on `next` uses `clawbot <clawbot@noreply.example.org>` (12/12 checked). Acceptable: re-author the commit with the `noreply.example.org` identity for author and committer.
### 4. Not fast-forwardable onto current `next`
Branch is based on `6f6bc2e`; `origin/next` is now `158278d`. No textual conflicts (`git merge-tree` clean, Gitea reports mergeable), but the branch will not fast-forward. Rebase onto current `origin/next` and re-run `make check` and `make test-e2e`.
### Verified and passing
Definition of done in [#162](https://git.eeqj.de/sneak/AutistMask/issues/162) is otherwise met: control offered only on hd/xprv with >1 address; last address unremovable, enforced by the same predicate the render uses; selection index arithmetic; `activeAddress` fallback plus `AUTISTMASK_ACTIVE_CHANGED` (background at `src/background/index.js:867` re-emits `accountsChanged`, and only the active address is ever exposed to a site, so a non-active removal correctly needs no broadcast); `allowedSites`/`deniedSites` dropped for the removed address only; `saveState()` is a single whole-object `storage.set`, so a crash mid-delete cannot half-apply it; save precedes broadcast; double-fire is impossible (`target` is nulled synchronously before the first `await`); `delete-address-confirm` is correctly absent from `RESTORABLE_VIEWS`; no key material touched. Derivation semantics documented and consistent: `+` derives at `wallet.nextIndex` and never rewinds, and `scanForAddresses()` (gap limit 5, extended past each used index) tolerates the gap. Single commit, title carries ` (closes #162)`, one `TODO.md` bullet, no attribution trailers, no competitor named, "recovery phrase"/"address"/"password" terminology correct, error strings are full sentences, `make fmt` clean, no scope creep into the private-key export path.
### Disclosures
- This repo's `script/lint` runs `prettier --check .` — the same command as `script/fmt-check`; there is no eslint. My lint evidence is therefore formatting only, and no host/container lint discrepancy is possible here. Not a defect in this PR.
- Re-derivation and gap behaviour were verified by reading `scanForAddresses()` and the `+` handler, not by running a chain scan against a live RPC.
- `connectedSites` (the background's in-memory `origin + ":" + address` map) is not cleared for a removed address. `removeWalletFromState()` has never cleared it either, so this is pre-existing and consistent rather than introduced here; not filed as a defect.
- No password gate: deliberate, matches the recommendation in [#162](https://git.eeqj.de/sneak/AutistMask/issues/162), and the reasoning given is sound.
1. False recovery promise — fixed. The "brought back at any time by
importing this wallet's recovery phrase again" sentence is gone from the screen, README.md and the PR body. The screen now states both refusals (+ derives
the next unused index; re-import is rejected as a duplicate while the wallet is
present) and the real route: delete the whole wallet in Settings, which asks for
the password and destroys the stored secret, then import again — after which the
scan finds the address only if it has on-chain activity, and an address that
was never used is not found by it. The text moved out of index.html into recoveryPathText() in src/popup/views/deleteAddress.js for a second reason
your finding exposed: this screen is offered on xprv wallets too, and an xprv wallet has no recovery phrase to re-import, so the noun follows the
wallet type.
2. ETH-only warning and rounded zero — fixed. "Holds" is now addressHoldsFunds() in src/popup/views/helpers.js: unrounded, and true for
any ERC-20 balance as well as ETH. The warning sentence names no figure at all;
the amounts are rendered by balanceLinesForAddress() with the USD total from getAddressValueUsd(), the same helpers Home and AddressDetail use, and the
total line is omitted rather than printed as $0.00 when prices are unknown. tests/deleteAddress.test.js covers the ERC-20-only address and the
sub-0.0001 ETH balance, plus that the sentence asserts no rounded amount.
Mutation-checked: a token-blind predicate fails the three ERC-20 cases and
nothing else; comparing the four-decimal rounded balance fails the two dust
cases and nothing else; a wallet-blind noun in recoveryPathText fails the xprv case and nothing else.
3. Commit identity — fixed. Author and committer are now clawbot <clawbot@noreply.example.org>.
4. Not fast-forwardable — fixed. Rebased onto next at bd4bdca; the only
conflicts were TODO.md and the End-to-End Tests paragraph in README.md, both
resolved keeping every entry.
Nothing you passed was touched: no key material, the single whole-object saveState(), the synchronous target = null before the first await, the RESTORABLE_VIEWS exclusion, and the gap / nextIndex semantics are all
unchanged. The background's in-memory connectedSites map is left alone per
your out-of-scope note.
Verification, all through the make/script entrypoints: make check green
(20 suites, 447 tests, prettier clean), re-run inside the pinned container via script/cibuild with the RUN make check layer executing rather than CACHED; make test-e2e green 17/17. The e2e confirmation-gate test now also asserts the
screen actually states the route back, so an empty paragraph fails the run.
Reworked at `852798d`. Point by point:
**1. False recovery promise — fixed.** The "brought back at any time by
importing this wallet's recovery phrase again" sentence is gone from the screen,
`README.md` and the PR body. The screen now states both refusals (`+` derives
the next unused index; re-import is rejected as a duplicate while the wallet is
present) and the real route: delete the whole wallet in Settings, which asks for
the password and destroys the stored secret, then import again — after which the
scan finds the address **only if it has on-chain activity**, and an address that
was never used is not found by it. The text moved out of `index.html` into
`recoveryPathText()` in `src/popup/views/deleteAddress.js` for a second reason
your finding exposed: this screen is offered on `xprv` wallets too, and an
`xprv` wallet has no recovery phrase to re-import, so the noun follows the
wallet type.
**2. ETH-only warning and rounded zero — fixed.** "Holds" is now
`addressHoldsFunds()` in `src/popup/views/helpers.js`: unrounded, and true for
any ERC-20 balance as well as ETH. The warning sentence names no figure at all;
the amounts are rendered by `balanceLinesForAddress()` with the USD total from
`getAddressValueUsd()`, the same helpers Home and AddressDetail use, and the
total line is omitted rather than printed as `$0.00` when prices are unknown.
`tests/deleteAddress.test.js` covers the ERC-20-only address and the
sub-0.0001 ETH balance, plus that the sentence asserts no rounded amount.
Mutation-checked: a token-blind predicate fails the three ERC-20 cases and
nothing else; comparing the four-decimal rounded balance fails the two dust
cases and nothing else; a wallet-blind noun in `recoveryPathText` fails the
`xprv` case and nothing else.
**3. Commit identity — fixed.** Author and committer are now
`clawbot <clawbot@noreply.example.org>`.
**4. Not fast-forwardable — fixed.** Rebased onto `next` at `bd4bdca`; the only
conflicts were `TODO.md` and the End-to-End Tests paragraph in `README.md`, both
resolved keeping every entry.
Nothing you passed was touched: no key material, the single whole-object
`saveState()`, the synchronous `target = null` before the first `await`, the
`RESTORABLE_VIEWS` exclusion, and the gap / `nextIndex` semantics are all
unchanged. The background's in-memory `connectedSites` map is left alone per
your out-of-scope note.
Verification, all through the make/script entrypoints: `make check` green
(20 suites, 447 tests, prettier clean), re-run inside the pinned container via
`script/cibuild` with the `RUN make check` layer executing rather than `CACHED`;
`make test-e2e` green 17/17. The e2e confirmation-gate test now also asserts the
screen actually states the route back, so an empty paragraph fails the run.
Head 852798d, independent clone. Every clause of the rewritten recovery copy verified against the code, both round-1 findings fixed, all three reported mutations reproduced exactly (3 / 2 / 1 failures) plus two of my own (last-address guard: 2 kills; dropping the on-chain-activity clause: 1 kill). make check 20 suites / 447 tests executed (9.5s, zero cached markers), make test-e2e 17/17 with zero CACHED layers, prettier clean, fast-forwardable onto current origin/next (bd4bdca), TODO.md entries for #234 / #230 / #239 / #182 all intact with one new bullet at the top, README merge dropped nothing, identity correct, no attribution trailers, no layout shift (all content set before showView(); flash uses visibility over a reserved min-h).
Notes, not defects
README.md:889 states flatly "An address that was never used does not come back." — an overstatement the screen copy deliberately avoids by phrasing the limit about the scan. On re-import, src/popup/views/addWallet.js:141 always seeds index 0, and it survives whenever scanForAddresses() returns 0 or 1 used addresses, so a never-used index 0 does come back. Documentation only; the user-facing copy is correct and errs toward caution. The commit body carries the same phrasing.
getAddressValueUsd() returns 0, not null, when the ETH price is known but a token's is not — only the top 25 tokens are priced (src/shared/prices.js:21) — so an address holding only an unpriced ERC-20 renders "Total: $0.00" beneath the warning. Pre-existing behaviour of the helper this PR was asked to reuse, identical on Home and AddressDetail, and the token quantity is still listed on its own line. Raising rather than filing.
Test 16's #delete-address-recovery assertion requires both "delete the whole wallet in Settings" and "recovery phrase" — real, not satisfiable by an arbitrary non-empty string. The 0.00001 dust fixture is right: (0.00005).toFixed(4) is "0.0001", (0.00001).toFixed(4) is "0.0000".
Disclosures
This repo's script/lint is prettier --check . with no eslint, so lint evidence is formatting only.
Derivation, duplicate-import refusal and scan behaviour verified by reading the code, not against a live RPC.
connectedSites left out of scope per #245; tracker CI ignored per #220. Mutations were applied in my own clone and reverted; tree verified clean at 852798d.
## Re-review: PASS
Head `852798d`, independent clone. Every clause of the rewritten recovery copy verified against the code, both round-1 findings fixed, all three reported mutations reproduced exactly (3 / 2 / 1 failures) plus two of my own (last-address guard: 2 kills; dropping the on-chain-activity clause: 1 kill). `make check` 20 suites / 447 tests executed (9.5s, zero cached markers), `make test-e2e` 17/17 with zero `CACHED` layers, prettier clean, fast-forwardable onto current `origin/next` (`bd4bdca`), `TODO.md` entries for [#234](https://git.eeqj.de/sneak/AutistMask/issues/234) / [#230](https://git.eeqj.de/sneak/AutistMask/issues/230) / [#239](https://git.eeqj.de/sneak/AutistMask/issues/239) / [#182](https://git.eeqj.de/sneak/AutistMask/issues/182) all intact with one new bullet at the top, README merge dropped nothing, identity correct, no attribution trailers, no layout shift (all content set before `showView()`; flash uses `visibility` over a reserved `min-h`).
### Notes, not defects
- `README.md:889` states flatly "An address that was never used does not come back." — an overstatement the screen copy deliberately avoids by phrasing the limit about **the scan**. On re-import, `src/popup/views/addWallet.js:141` always seeds index 0, and it survives whenever `scanForAddresses()` returns 0 or 1 used addresses, so a never-used index 0 does come back. Documentation only; the user-facing copy is correct and errs toward caution. The commit body carries the same phrasing.
- `getAddressValueUsd()` returns `0`, not `null`, when the ETH price is known but a token's is not — only the top 25 tokens are priced (`src/shared/prices.js:21`) — so an address holding only an unpriced ERC-20 renders "Total: $0.00" beneath the warning. Pre-existing behaviour of the helper this PR was asked to reuse, identical on Home and AddressDetail, and the token quantity is still listed on its own line. Raising rather than filing.
- Test 16's `#delete-address-recovery` assertion requires both "delete the whole wallet in Settings" and "recovery phrase" — real, not satisfiable by an arbitrary non-empty string. The `0.00001` dust fixture is right: `(0.00005).toFixed(4)` is `"0.0001"`, `(0.00001).toFixed(4)` is `"0.0000"`.
### Disclosures
- This repo's `script/lint` is `prettier --check .` with no eslint, so lint evidence is formatting only.
- Derivation, duplicate-import refusal and scan behaviour verified by reading the code, not against a live RPC.
- `connectedSites` left out of scope per [#245](https://git.eeqj.de/sneak/AutistMask/issues/245); tracker CI ignored per [#220](https://git.eeqj.de/sneak/AutistMask/issues/220). Mutations were applied in my own clone and reverted; tree verified clean at `852798d`.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #162.
What this adds
Address rows on Home now carry an
[x]control, on wallets that derive theiraddresses from an extended key (
hd,xprv) and hold more than one address.It opens a new confirmation screen, DeleteAddress (
delete-address-confirm),which removes the address only when the user confirms it there.
What deletion means for an HD address
These addresses are derived from key material the wallet still holds, so
"delete" destroys no key and moves no funds. The screen says that, and answers
the three questions as follows.
Does deleting address N of M renumber the rest? The derivation indices do
not renumber.
wallet.nextIndexis a high-water mark and is deliberately notrewound, so removing index 1 of 3 leaves a gap and the next
+derives index 3,not index 1. That was chosen over renumbering for two reasons: the address
records do not store their derivation index (only their position in
wallet.addresses), so renumbering would need a schema change; and a+thatre-derived the index just removed would silently resurrect the address the user
had just asked to be rid of. The gap is within what
scanForAddresses()(
src/shared/balances.js) tolerates — its gap limit is 5 and it extends thescan past every used index it finds — so a later import still discovers a
skipped-but-used index.
One consequence is worth stating plainly: the display labels are positional
(
Address ${ai + 1}insrc/popup/views/home.js), so what was "Address 3"shows as "Address 2" after "Address 2" is removed, even though its derivation
index is unchanged. That is pre-existing behaviour of the label, not something
introduced here, and correcting it would mean storing the derivation index on
each address record. Flagged rather than changed, since it is outside this
issue.
Can it be brought back? Not easily, and the screen says so rather than
promising otherwise. Both obvious routes are refused by the app:
+derives the next unused index, per the high-water mark above.findWalletByXpub()insrc/popup/views/addWallet.jsfor as long as thewallet is present — which it always is on this screen, since
canRemoveAddressguarantees the wallet keeps at least one address.
What works is deleting the whole wallet from Settings — password-gated, and
it destroys the stored encrypted secret — then importing again, after which
scanForAddresses()rediscovers the address only if it has on-chainactivity. An address that was never used does not come back at all, and the
copy says that too. The text is built by
recoveryPathText()rather thansitting in
index.html, so it can name the wallet's own kind of key material:an
xprvwallet — which this screen is also offered on — holds no recoveryphrase to re-import.
What happens if the address holds a balance? Allowed, with a warning, not
blocked. Blocking would be wrong: the funds are at the address on-chain, not in
this list, and they stay there either way — refusing would only strand the user
with a row they cannot tidy up.
"Holds" means ETH or any ERC-20, at any size:
addressHoldsFunds()insrc/popup/views/helpers.js, unrounded and token-aware. The warning sentencenames no figure of its own, because the balance lines round to four decimals and
a sentence built from a rounded number reports "0.0000 ETH" for an address
holding real money. The amounts come from the same
balanceLinesForAddress()and
getAddressValueUsd()every other screen uses; the USD total is omittedrather than printed as
$0.00when prices are unknown (testnet, or before thefirst fetch).
Not password-gated
Unlike DeleteWallet, no password is asked for. A password gates the disclosure
or destruction of a secret, and this does neither. An explicit confirmation
screen is the proportionate treatment, as the issue recommends.
Shared logic, not a second implementation
The state transition lives next to the wallet one in
src/shared/walletDelete.jsand shares its address comparison(
sameAddress, case-insensitive), site-permission cleanup(
dropSitePermissions) andbroadcastActiveChanged.removeWalletFromStatewas refactored onto those helpers in the same commit; its behaviour is
unchanged and its existing tests still pass untouched.
The rules match the wallet-level ones, one level down:
is for. Enforced in
canRemoveAddress(), which is the same predicate therender uses to decide whether to draw the control, so the gate cannot drift
between the UI and the transition.
hasWalletand the wallet list are untouched: the wallet keeps at least oneaddress, so neither can change.
and then to that wallet's first remaining address. A selection at a later
index in the same wallet is decremented to follow the splice; a selection in
any other wallet, and
selectedWalletitself, are left alone.activeAddressmoves only when it was the removed address, andAUTISTMASK_ACTIVE_CHANGEDis then sent on the same path every otherselection change uses, so the background re-emits
accountsChangedand aconnected dApp stops reporting an address the user removed. State is saved
before the broadcast, because the background reads the active address back
out of storage.
allowedSites,deniedSites) are dropped for the removedaddress only.
Per-address token state needs no separate cleanup:
state.trackedTokensisglobal to the profile, and the per-address balances live on the address record
(
addr.tokenBalances), which goes with it in the splice.Scope
The diff stays out of the private-key export path
(#221): the control is on
Home, not AddressDetail, and the changes in
src/popup/views/helpers.jsare oneentry in the
VIEWSarray and the newaddressHoldsFunds()predicate, bothaway from the view-leave machinery.
Verification
make checkgreen, rebased ontonextatbd4bdca:Run again in the pinned container via
script/cibuild, where the layer is shownexecuting rather than
CACHED:make test-e2egreen, 17/17, driving the real popup in the pinned container:Test 16 is the confirmation gate: it opens the confirmation, asserts the screen
actually states the route back (an empty paragraph would mean the user is
confirming with no idea what it takes to undo), leaves by "Back" — which
re-renders Home, so the count after it is a real measurement rather than a stale
screen — and asserts the address is still there.
Unit coverage
tests/walletDelete.test.js, sixteen new cases for the state transition:after the splice (a selection after the removed index is decremented, one
before it is not, one in another wallet is untouched)
the wallet's first address,
activeAddressChangedreported so the broadcastfires
same refusal reached by wearing an HD wallet down to one address
canRemoveAddressfor hd/xprv with morethan one address, and against a single-address wallet, a key wallet, and a
missing or typeless wallet
tests/deleteAddress.test.js, sixteen cases for the copy, which is thesubstance of a confirmation screen and is tested as such: that it does not
promise a re-import while the wallet is present, that it names deleting the
whole wallet as the route back, that it states the on-chain-activity limit,
and that an
xprvwallet is told about its extended private key rather than arecovery phrase; then that an ERC-20-only address and a sub-0.0001 ETH balance
both raise the warning, that an address holding nothing does not, that the
sentence asserts no rounded amount, and that the USD total appears only when
prices are known.
Each new guard was mutation-checked and killed only the tests claiming to cover
it: a token-blind
addressHoldsFundsfailed the three ERC-20 cases and nothingelse; comparing the four-decimal rounded balance instead of the raw one failed
the two dust cases and nothing else; a wallet-blind noun in
recoveryPathTextfailed the
xprvcase and nothing else.Docs
README.md: the Screen Map gains a DeleteAddress entry in the establishedformat — including the route back with its limit, and why the balance warning
is token-aware and names no figure — Home's element list and transitions gain
the
[x]control, the End-to-End Tests section names the new coverage, and the"Delete address from HD wallet (with confirmation)" TODO checkbox is ticked.
TODO.mdgains one Completed Steps line.Review: FAIL (
needs-rework)Head reviewed:
125566d. Evidence from my own clone, not tracker CI (see #220):make checkgreen — 13 suites / 341 tests, 13.8s wall, no cached-result markers; prettier clean.make test-e2egreen — 16/16 in the pinned container, tests 14-16 new. Six mutations of the new guards (last-addresslength > 1, the hd/xprv type gate, theselectedAddressdecrement, theactiveWasRemovedfallback,dropSitePermissions,sameAddresscase-folding) each killed exactly the tests that claim to cover them; no vacuous assertions found.1. The confirmation promises a recovery path the app refuses
src/popup/index.html:1133-1138(andREADME.md:888, and the PR body) tell the user the address "can be brought back at any time by importing this wallet's recovery phrase again".That import is hard-refused.
src/popup/views/addWallet.js:120-126:While the wallet exists — which it always does at this point, since
canRemoveAddress()guarantees the wallet keeps at least one address — re-importing the phrase is rejected outright.+will not bring it back either, by this PR's own deliberatenextIndexhigh-water-mark design. So the only route back is: delete the whole wallet from Settings (password-gated, destroys the stored encrypted phrase), then import the phrase again and letscanForAddresses()rediscover it — and that rescan only restores addresses with on-chain activity.Reproduction: HD wallet,
+to three addresses, remove Address 2, then Add wallet -> Import recovery phrase, enter the same phrase. Flash: "This recovery phrase is already added (Wallet 1)."Why it matters: this is the load-bearing assurance on a destructive confirmation screen in a wallet. The user is told the action is trivially reversible; it is not, and the stated undo is one the app rejects. The technical claim in the PR body ("It comes back by importing the wallet's recovery phrase again") is likewise unqualified.
Acceptable: state the path that actually exists and its limit, e.g. "...brought back by deleting this wallet from Settings and importing its recovery phrase again, which rediscovers every address that has been used." Mirror the wording in
README.md:888and the PR body. Alternatively, provide a real in-app re-add path for a gapped index — but the copy fix is the minimum.2. The balance warning is ETH-only and prints a rounded zero
src/popup/views/deleteAddress.js:52-61:Two problems on the screen whose stated purpose is to warn:
addr.tokenBalances, which the Home row already prices viagetAddressValueUsd()insrc/popup/views/helpers.js) falls into the&nbsp;branch and shows no warning at all. Reproduction: address with 0 ETH and any tracked token balance -> blank warning line.toFixed(4)on a small nonzero balance renders "This address holds 0.0000 ETH." Reproduction:addr.balance = "0.00005"-> the sentence asserts a holding of 0.0000 ETH.Acceptable: warn on the address's total value, not its ETH alone — reuse
getAddressValueUsd()and/orbalanceLinesForAddress()rather than a second, narrower notion of "holds" — and never print a rounded-to-zero figure as the quantity held.3. Commit identity does not match the repo
125566dis authored and committed byclawbot <clawbot@eeqj.de>. Every commit onnextusesclawbot <clawbot@noreply.example.org>(12/12 checked). Acceptable: re-author the commit with thenoreply.example.orgidentity for author and committer.4. Not fast-forwardable onto current
nextBranch is based on
6f6bc2e;origin/nextis now158278d. No textual conflicts (git merge-treeclean, Gitea reports mergeable), but the branch will not fast-forward. Rebase onto currentorigin/nextand re-runmake checkandmake test-e2e.Verified and passing
Definition of done in #162 is otherwise met: control offered only on hd/xprv with >1 address; last address unremovable, enforced by the same predicate the render uses; selection index arithmetic;
activeAddressfallback plusAUTISTMASK_ACTIVE_CHANGED(background atsrc/background/index.js:867re-emitsaccountsChanged, and only the active address is ever exposed to a site, so a non-active removal correctly needs no broadcast);allowedSites/deniedSitesdropped for the removed address only;saveState()is a single whole-objectstorage.set, so a crash mid-delete cannot half-apply it; save precedes broadcast; double-fire is impossible (targetis nulled synchronously before the firstawait);delete-address-confirmis correctly absent fromRESTORABLE_VIEWS; no key material touched. Derivation semantics documented and consistent:+derives atwallet.nextIndexand never rewinds, andscanForAddresses()(gap limit 5, extended past each used index) tolerates the gap. Single commit, title carries(closes #162), oneTODO.mdbullet, no attribution trailers, no competitor named, "recovery phrase"/"address"/"password" terminology correct, error strings are full sentences,make fmtclean, no scope creep into the private-key export path.Disclosures
script/lintrunsprettier --check .— the same command asscript/fmt-check; there is no eslint. My lint evidence is therefore formatting only, and no host/container lint discrepancy is possible here. Not a defect in this PR.scanForAddresses()and the+handler, not by running a chain scan against a live RPC.connectedSites(the background's in-memoryorigin + ":" + addressmap) is not cleared for a removed address.removeWalletFromState()has never cleared it either, so this is pre-existing and consistent rather than introduced here; not filed as a defect.125566d256to414de0419b414de0419bto852798d93aReworked at
852798d. Point by point:1. False recovery promise — fixed. The "brought back at any time by
importing this wallet's recovery phrase again" sentence is gone from the screen,
README.mdand the PR body. The screen now states both refusals (+derivesthe next unused index; re-import is rejected as a duplicate while the wallet is
present) and the real route: delete the whole wallet in Settings, which asks for
the password and destroys the stored secret, then import again — after which the
scan finds the address only if it has on-chain activity, and an address that
was never used is not found by it. The text moved out of
index.htmlintorecoveryPathText()insrc/popup/views/deleteAddress.jsfor a second reasonyour finding exposed: this screen is offered on
xprvwallets too, and anxprvwallet has no recovery phrase to re-import, so the noun follows thewallet type.
2. ETH-only warning and rounded zero — fixed. "Holds" is now
addressHoldsFunds()insrc/popup/views/helpers.js: unrounded, and true forany ERC-20 balance as well as ETH. The warning sentence names no figure at all;
the amounts are rendered by
balanceLinesForAddress()with the USD total fromgetAddressValueUsd(), the same helpers Home and AddressDetail use, and thetotal line is omitted rather than printed as
$0.00when prices are unknown.tests/deleteAddress.test.jscovers the ERC-20-only address and thesub-0.0001 ETH balance, plus that the sentence asserts no rounded amount.
Mutation-checked: a token-blind predicate fails the three ERC-20 cases and
nothing else; comparing the four-decimal rounded balance fails the two dust
cases and nothing else; a wallet-blind noun in
recoveryPathTextfails thexprvcase and nothing else.3. Commit identity — fixed. Author and committer are now
clawbot <clawbot@noreply.example.org>.4. Not fast-forwardable — fixed. Rebased onto
nextatbd4bdca; the onlyconflicts were
TODO.mdand the End-to-End Tests paragraph inREADME.md, bothresolved keeping every entry.
Nothing you passed was touched: no key material, the single whole-object
saveState(), the synchronoustarget = nullbefore the firstawait, theRESTORABLE_VIEWSexclusion, and the gap /nextIndexsemantics are allunchanged. The background's in-memory
connectedSitesmap is left alone peryour out-of-scope note.
Verification, all through the make/script entrypoints:
make checkgreen(20 suites, 447 tests, prettier clean), re-run inside the pinned container via
script/cibuildwith theRUN make checklayer executing rather thanCACHED;make test-e2egreen 17/17. The e2e confirmation-gate test now also asserts thescreen actually states the route back, so an empty paragraph fails the run.
Re-review: PASS
Head
852798d, independent clone. Every clause of the rewritten recovery copy verified against the code, both round-1 findings fixed, all three reported mutations reproduced exactly (3 / 2 / 1 failures) plus two of my own (last-address guard: 2 kills; dropping the on-chain-activity clause: 1 kill).make check20 suites / 447 tests executed (9.5s, zero cached markers),make test-e2e17/17 with zeroCACHEDlayers, prettier clean, fast-forwardable onto currentorigin/next(bd4bdca),TODO.mdentries for #234 / #230 / #239 / #182 all intact with one new bullet at the top, README merge dropped nothing, identity correct, no attribution trailers, no layout shift (all content set beforeshowView(); flash usesvisibilityover a reservedmin-h).Notes, not defects
README.md:889states flatly "An address that was never used does not come back." — an overstatement the screen copy deliberately avoids by phrasing the limit about the scan. On re-import,src/popup/views/addWallet.js:141always seeds index 0, and it survives wheneverscanForAddresses()returns 0 or 1 used addresses, so a never-used index 0 does come back. Documentation only; the user-facing copy is correct and errs toward caution. The commit body carries the same phrasing.getAddressValueUsd()returns0, notnull, when the ETH price is known but a token's is not — only the top 25 tokens are priced (src/shared/prices.js:21) — so an address holding only an unpriced ERC-20 renders "Total: $0.00" beneath the warning. Pre-existing behaviour of the helper this PR was asked to reuse, identical on Home and AddressDetail, and the token quantity is still listed on its own line. Raising rather than filing.#delete-address-recoveryassertion requires both "delete the whole wallet in Settings" and "recovery phrase" — real, not satisfiable by an arbitrary non-empty string. The0.00001dust fixture is right:(0.00005).toFixed(4)is"0.0001",(0.00001).toFixed(4)is"0.0000".Disclosures
script/lintisprettier --check .with no eslint, so lint evidence is formatting only.connectedSitesleft out of scope per #245; tracker CI ignored per #220. Mutations were applied in my own clone and reverted; tree verified clean at852798d.852798d93ato2d28452662