test: containerized Firefox end-to-end harness (closes #184)

Drives the real popup in a real Firefox with dist/firefox/ installed as an
unpacked MV2 temporary add-on, via geckodriver. Covers popup load, wallet
creation through the UI, and the Add Token screen. Outside make check, like
the Chrome suite.

Zero npm dependencies: tests/e2e/firefox/driver.js is a WebDriver client
over global fetch and child_process against geckodriver's HTTP API. The
Dockerfile pins the node base image, the Firefox 153.0.3 tarball and
geckodriver 0.36.0 by digest.

Errors are read from the privileged nsIConsoleService in Marionette's chrome
context, filtered to non-warning entries whose sourceName is the extension
origin. BiDi log.entryAdded delivers nothing at all for extension pages, so
a Playwright-BiDi or Puppeteer-BiDi harness would see nothing and report
success; the code says so where someone would be tempted to simplify it.
Errors logged during add-on install and background startup are drained and
folded into step 1, never discarded: a throw at the top of
src/background/index.js kills the background page and fails the run.
Content-script capture is left as unverified, because --network none leaves
no http:// page for a content script to be injected into.

Each drain reads the console and clears it in ONE chrome script. Splitting
the read from Services.console.reset() left a window between the two round
trips in which an error was logged into a buffer about to be discarded, and
destroyed unread rather than deferred to the next drain; a probe of 100
sequenced throws at 20ms spacing lost one. With the drain atomic the same
probe accounts for every throw that falls inside the observed window, on two
consecutive runs.

No driver layer is shared with the Chrome suite and the three UI steps are
written twice deliberately: the two backends have no common substrate, and
three steps do not pay for a shim.

Two limits are documented rather than papered over. Error capture is
poll-based, so an error is attributed to a step and not to a moment within
it, and the observed window ends ~1.5s after the last step returns, measured:
errors at +0.5s, +1.0s and +1.5s are reported and +1.6s and later never are,
because the browser is torn down. Nothing is stubbed; the container runs with
--network none instead, which proves no request escaped, cannot report which
were attempted, and runs only the failure branches of network-dependent code.

Makefile

@@ -1,4 +1,4 @@
-.PHONY: bootstrap setup install test test-e2e lint fmt fmt-check check docker hooks build build-debug verify-build clean dev
+.PHONY: bootstrap setup install test test-e2e test-e2e-firefox lint fmt fmt-check check docker hooks build build-debug verify-build clean dev
 
 # Standard targets are thin shims; the implementations live in script/
 # per the scripts-to-rule-them-all pattern (see the Entrypoints section
@@ -16,10 +16,13 @@ install:
 test:
 	@script/test
 
-# Browser end-to-end suite. Requires docker; not part of check.
+# Browser end-to-end suites. Both require docker; neither is part of check.
 test-e2e:
 	@script/test-e2e
 
+test-e2e-firefox:
+	@script/test-e2e-firefox
+
 lint:
 	@script/lint
 

README.md

@@ -83,18 +83,29 @@ provide:
   git pre-commit hook
 - `script/projectname` — print the project name (used for the Docker image tag)
 - `script/test` — run the test suite (jest)
-- `script/test-e2e` — run the browser end-to-end suite (docker required; see
+- `script/test-e2e` — run the Chrome browser end-to-end suite (docker required;
+  see [End-to-End Tests](#end-to-end-tests))
+- `script/test-e2e-firefox` — run the Firefox browser end-to-end suite (docker
+  required; builds its own pinned image, see
   [End-to-End Tests](#end-to-end-tests))
 - `script/lint` — run the linter
 - `script/fmt` — format all files (writes)
 - `script/fmt-check` — check formatting (read-only)
-- `script/check` — run test, lint, and fmt-check
+- `script/check` — run test, test-verify-build, lint, and fmt-check
 - `script/verify-build` — assert the compiled `DEBUG` state of the bundles in
   `dist/`: every bundle containing `src/shared/constants.js` must have `DEBUG`
   off, or on when `AUTISTMASK_DEBUG=1`. Run automatically at the end of
   `make build` and `make build-debug`; fails loudly rather than passing if it
   cannot determine a bundle's state. Not part of `make check`, which does not
   depend on build artifacts existing.
+- `script/test-verify-build` — exercise every failure mode of
+  `script/verify-build` against a fixture tree in a temp dir, asserting the exit
+  status and the message of each. Part of `make check`; it reads no build
+  artifacts and writes nothing under `dist/`. The cases that depend on file
+  permissions cannot mean anything for a process that is not subject to them, so
+  the harness proves its runner against a mode-000 file before counting them,
+  dropping to an unprivileged user when run as root; if it cannot, it skips
+  those cases and says so in a banner rather than passing them.
 - `script/docker` — build the Docker image tagged via `script/projectname`
 - `script/cibuild` — CI entrypoint: plain `docker build .`
 - `script/precommit` — run by the git pre-commit hook; runs `script/check`
@@ -115,6 +126,14 @@ The Makefile shims to those. It also carries a few targets that have no
 
 ## End-to-End Tests
 
+There are two suites, one per browser, and they share no code. Chrome runs on
+Playwright; Firefox has its own WebDriver client, because Playwright cannot
+observe errors on a Firefox extension page at all — see
+[Firefox](#firefox-make-test-e2e-firefox) below. Both require docker, and both
+are outside `make check`.
+
+### Chrome (`make test-e2e`)
+
 `make test-e2e` builds `dist/chrome/` and drives the **real popup in a real
 Chrome**, loaded as an unpacked MV3 extension inside a pinned
 `mcr.microsoft.com/playwright` container (pinned by digest in `script/test-e2e`;
@@ -130,8 +149,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
+land on it. It also covers address removal: which wallets offer the control at
-from fixtures in `tests/e2e/network.js`, so the run is deterministic and fully
+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.
 
@@ -172,12 +194,82 @@ a `ReferenceError` from a used-but-not-imported identifier is invisible to
 `make check` (`script/lint` is only `prettier --check`) but fatal in a browser,
 and this suite exists because exactly that class of bug shipped twice.
 
-`make test-e2e` is deliberately **not** part of `make check` or `make test`.
+### Firefox (`make test-e2e-firefox`)
-`REPO_POLICIES.md` caps `make test` at 20 seconds and a browser suite does not
+
-fit; nothing in `tests/e2e/` is named `*.test.js`, so jest cannot pick it up
+`make test-e2e-firefox` builds `dist/firefox/` and drives the **real popup in a
-either. It is also not wired into the Gitea workflow yet — docker-in-docker in
+real Firefox**, installed as an unpacked MV2 temporary add-on via geckodriver.
-CI is a separate question. Run it locally before changing anything under
+It covers popup load, wallet creation through the UI, and the Add Token screen.
-`src/popup/views/`.
+The suite lives in `tests/e2e/firefox/` and has **no npm dependencies at all**:
+it is a small WebDriver client built on global `fetch` and `child_process`
+against geckodriver's HTTP API.
+
+Unlike the Chrome suite it builds its own container image rather than pulling a
+published one, because no published image carries both a pinned Firefox and a
+matching geckodriver. `tests/e2e/firefox/Dockerfile` pins all three external
+artifacts by digest — the `node` base image, the Firefox 153.0.3 tarball, and
+geckodriver 0.36.0 — and the Firefox version in particular must not float:
+`-remote-allow-system-access` is **mandatory** on 153 and was not on 142.
+Without that flag, both navigating to `moz-extension://` and running
+chrome-context script fail with `unsupported operation`. The flag grants the
+driver full chrome privileges over that browser, which is acceptable only
+because it is a throwaway container.
+
+The popup's `moz-extension://` uuid is **pinned, not discovered**: the profile
+pref `extensions.webextensions.uuids` maps the extension id that
+`manifest/firefox.json` already declares to a fixed uuid, so the popup URL is
+deterministic. Navigation uses **classic** WebDriver `POST /session/{id}/url`,
+because BiDi's `browsingContext.navigate` refuses `moz-extension://` outright.
+
+**Any uncaught error from a `moz-extension://` source fails the run**, including
+errors from the background page, which the suite never navigates to: a `throw`
+at the top of `src/background/index.js` kills the background page and fails
+step 1. Content-script errors should arrive by the same route, but this suite
+does not exercise it and does not claim it — with `--network none` there is no
+`http://` page for a content script to be injected into. Errors from add-on
+install and background startup are folded into step 1 rather than discarded.
+Errors are read from the privileged `nsIConsoleService` in Marionette's chrome
+context and filtered to non-warning entries whose `sourceName` is the extension
+origin. That mechanism is not a stylistic choice. WebDriver BiDi's
+`log.entryAdded` delivers **nothing** for extension pages: on a plain `http://`
+page it reports uncaught errors with stack traces, and on the `moz-extension://`
+popup it reports zero events, because Firefox's remote agent excludes extension
+browsing contexts from BiDi observation. Any harness built on Playwright-BiDi or
+Puppeteer-BiDi would therefore see nothing and report success, which is exactly
+the vacuous check this repo has already shipped twice. Do not migrate this suite
+to BiDi.
+
+Two limits are worth knowing, both real differences from the Chrome suite:
+
+- **Error capture is poll-based, not event-streamed.** The console is drained at
+  each step boundary, so an error is attributed to the step it was drained
+  after, not to a moment within it. The window that is drained runs from add-on
+  install to **≈1.5s** after the last step returns, then the browser is torn
+  down; measured with throws scheduled at fixed offsets, errors at +0.5s, +1.0s
+  and +1.5s are reported and +1.6s and later never are. Within that window
+  nothing is dropped — each drain reads and clears the console in a single
+  chrome round trip, so an error logged mid-drain lands in that batch or the
+  next one rather than being destroyed unread; a probe of 100 throws at 20ms
+  spacing accounts for every one that falls inside the window, twice running.
+  What poll-based costs is location, not coverage: an error cannot be placed
+  within a step the way the Chrome suite's `pageerror` events place it.
+- **Nothing is stubbed, which inverts the coverage of network-dependent code.**
+  There is no fixture layer; the container runs with `--network none` instead,
+  so the run is offline and deterministic and no request can escape. The
+  extension swallows its own fetch failures, so the flows are unaffected — but
+  every network call fails, so only the _failure_ branches of code that depends
+  on one are ever executed. A `ReferenceError` in the success path of
+  `renderTransactions`, or of price or balance rendering, passes this suite
+  green. The offline run is also weaker than the Chrome suite's interception: it
+  proves nothing got out, but it cannot report which requests were attempted.
+  Closing that gap needs a fixture layer, deliberately out of scope for this
+  harness.
+
+Neither `make test-e2e` nor `make test-e2e-firefox` is part of `make check` or
+`make test`. `REPO_POLICIES.md` caps `make test` at 20 seconds and a browser
+suite does not fit; nothing in `tests/e2e/` is named `*.test.js`, so jest cannot
+pick it up either. Neither is wired into the Gitea workflow yet —
+docker-in-docker in CI is a separate question. Run them locally before changing
+anything under `src/popup/views/`.
 
 ## Rationale
 
@@ -221,6 +313,7 @@ src/
     prices.js         — ETH/USD and token/USD via CoinDesk API
     scamlist.js       — known fraud contract addresses
     state.js          — persisted state (extension storage)
+    symbolSpoof.js    — the known-symbol spoof rule, shared by all surfaces
     tokenList.js      — top ERC-20 tokens by market cap (hardcoded)
     transactions.js   — tx history fetching + anti-poisoning filters
     uniswap.js        — Uniswap Universal Router calldata decoder
@@ -450,11 +543,12 @@ Which tokens an address shows is decided by `fetchTokenBalances()` in
 tokens do appear without the user adding them. An ERC-20 is shown when its
 balance is nonzero and it is in the bundled known-token list, is tracked by the
 user, or has 1,000 or more holders; a token claiming a symbol from the bundled
-list from any other contract address is always dropped. That filter is
+list from any other contract address is always dropped, and so is any token
-unconditional — the "Hide tokens with fewer than 1,000 holders" setting governs
+claiming a symbol that belongs to the native asset and therefore has no
-the transaction history and the send-screen token selector, not this list.
+legitimate contract at all (`"ETH"`). That filter is unconditional — the "Hide
-Tracked tokens with a zero balance are listed as well while "Show tracked tokens
+tokens with fewer than 1,000 holders" setting governs the transaction history
-with zero balance" is on.
+and the send-screen token selector, not this list. Tracked tokens with a zero
+balance are listed as well while "Show tracked tokens with zero balance" is on.
 
 #### Navigation
 
@@ -521,8 +615,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
+      `[info]` button, an `[x]` button (only on HD and xprv wallets holding more
-      each token shown for that address
+      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
@@ -532,6 +627,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**
@@ -910,6 +1006,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
@@ -1272,14 +1417,15 @@ indexes it as a real token transfer.
   that is the only thing that populates it. In the transaction history the check
   is the "Hide fake tokens impersonating a known symbol" setting, on by default;
   with it off, spoofed transfers are shown and no new blocklist entries are
-  learned from them. The send-screen token selector applies the same check
+  learned from them. The send-screen token selector and the balance list apply
-  unconditionally, because it decides which tokens the user can act on rather
+  the same check unconditionally, because they decide which tokens the user can
-  than what the history displays. The balance list applies it unconditionally
+  act on and what the user believes they own rather than what the history
-  too, but not identically: it exempts symbols that `KNOWN_SYMBOLS` maps to
+  displays. All three surfaces read the rule from `src/shared/symbolSpoof.js`,
-  `null`, and `"ETH"` is the only one. So the fake "Ethereum" token above is
+  so they cannot answer the question differently. A symbol the list maps to no
-  filtered from the transaction history and from the send selector, but a
+  contract at all — `"ETH"`, the native asset, is the only one — may be borne by
-  fake-`ETH` ERC-20 that clears the balance list's own 1,000-holder floor — or
+  no contract, so every ERC-20 claiming it is a spoof on all three. The user's
-  that the user tracked manually — is still shown in the balance list.
+  real ETH balance is not an ERC-20 and is read over RPC, so the rule never sees
+  it.
 
 - **Low-holder token filtering**: Token transfers from ERC-20 contracts with
   fewer than 1,000 holders are hidden from transaction history by default.
@@ -1315,13 +1461,12 @@ indexes it as a real token transfer.
   a sharp tool — users who understand the risks can configure the wallet to show
   everything unfiltered, unix-style. All four settings govern the transaction
   history; what else each one reaches varies. The known-symbol check also runs
-  unconditionally on the send-screen token selector, and on the balance list
+  unconditionally on the send-screen token selector and on the balance list, in
-  except for symbols mapped to `null` (`"ETH"` alone), which the balance list
+  both cases identically to the history. The fraud contract blocklist is applied
-  does not filter. The fraud contract blocklist is applied unconditionally on
+  unconditionally on that selector and is not consulted by the balance list at
-  that selector and is not consulted by the balance list at all. The low-holder
+  all. The low-holder setting also gates the send selector, while the balance
-  setting also gates the send selector, while the balance list's own
+  list's own 1,000-holder floor is unconditional (see Data Model). The dust
-  1,000-holder floor is unconditional (see Data Model). The dust threshold
+  threshold applies to the transaction history alone.
-  applies to the transaction history alone.
 
 #### Phishing Domain Protection
 
@@ -1393,7 +1538,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

TODO.md

@@ -30,9 +30,10 @@ compiled off.
 
 The backlog lives on the
 [Gitea tracker](https://git.eeqj.de/sneak/AutistMask/issues), which is
-authoritative; this file does not duplicate it. Full policy file set present. A
+authoritative; this file does not duplicate it. Full policy file set present.
-real-browser end-to-end suite (`make test-e2e`) now sits alongside `make check`,
+Real-browser end-to-end suites (`make test-e2e` for Chrome,
-which cannot see a runtime `ReferenceError` in a popup view.
+`make test-e2e-firefox` for Firefox) now sit alongside `make check`, which
+cannot see a runtime `ReferenceError` in a popup view.
 
 # Next Step
 
@@ -44,6 +45,50 @@ undefined identifiers, which is how
 
 # Completed Steps
 
+- 2026-08-12: A containerized Firefox end-to-end harness
+  (`make test-e2e-firefox`) drives the real popup in a real Firefox with the MV2
+  build installed as a temporary add-on. Zero npm dependencies — a WebDriver
+  client over `fetch` against geckodriver — with `node`, Firefox 153.0.3 and
+  geckodriver 0.36.0 all pinned by digest. Uncaught errors are read from the
+  privileged console service in Marionette's chrome context, because BiDi
+  `log.entryAdded` reports nothing at all for extension pages; each drain reads
+  and clears the console in one chrome round trip, so nothing logged between two
+  drains is destroyed unread, and errors logged during add-on install and
+  background startup are folded into step 1 instead of being cleared.
+  Demonstrated discriminating by exiting 1 on a `throw` at the top of
+  `src/background/index.js`, on a build with one import removed, on a
+  `setTimeout` throw whose UI assertions all pass, on an unhandled
+  `Promise.reject` and on an undefined identifier in `home.js`, and 0 on the
+  branch as it stands ([#184](https://git.eeqj.de/sneak/AutistMask/issues/184)).
+- 2026-08-12: Approval verification became an allowlist — transaction type
+  restricted to 0/1/2 so an EIP-7702 delegation can no longer ride along on an
+  approved transfer, every consequential field compared, the artifact
+  re-serialized from the checked fields alone and its exact bytes required to be
+  the canonical encoding of what was broadcast. One approval now yields at most
+  one broadcast, and every path that retires a pending approval — popup close,
+  active-address change, a late reject — goes through a single chokepoint that
+  refuses to settle an attempt already claimed for signing and broadcast
+  ([#174](https://git.eeqj.de/sneak/AutistMask/issues/174)).
+- 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
+  transaction history and the Send selector but listed as a holding named ETH. A
+  symbol with no legitimate contract may now be borne by no contract on any of
+  the three surfaces, and the native exemption is "has no contract address", so
+  a second null-mapped symbol needs no call-site change. The user's real ETH
+  balance is read over RPC and never passes through the rule
+  ([#235](https://git.eeqj.de/sneak/AutistMask/issues/235)).
+- 2026-08-12: `script/verify-build`'s failure modes are now a committed target,
+  `script/test-verify-build`, run by `make check`. It asserts the exit status
+  and the message of every case against a fixture tree in a temp dir, and drops
+  privileges (proving the runner against a mode-000 file first) for the cases
+  that only mean something when file permissions are in force
+  ([#227](https://git.eeqj.de/sneak/AutistMask/issues/227)).
 - 2026-08-12: WaitTx lifecycle: a receipt and the 60-second timeout can no
   longer both render on one tick, no timer or in-flight lookup outlives its
   wait, a failed receipt lookup no longer counts as a timeout (but six in a row
@@ -202,9 +247,9 @@ tracker.
 - Pre-1.0 security review of the extension (key handling, DEBUG mode policy, RPC
   input validation) before any 1.0rc tag. Individual filed issues are parts of
   it, but the review is broader than any of them.
-- Decide whether docker-in-docker makes `make test-e2e` runnable in the Gitea
+- Decide whether docker-in-docker makes `make test-e2e` and
-  workflow. Extending the suite itself is tracked as
+  `make test-e2e-firefox` runnable in the Gitea workflow. Extending the Chrome
-  [#183](https://git.eeqj.de/sneak/AutistMask/issues/183) and
+  suite itself is tracked as
-  [#184](https://git.eeqj.de/sneak/AutistMask/issues/184).
+  [#183](https://git.eeqj.de/sneak/AutistMask/issues/183).
 - Cut 1.0.0 once the milestone is empty, then continue tagging as milestones
   land.

script/check

@@ -1,12 +1,13 @@
 #!/bin/sh
-# script/check: run all checks (test, lint, fmt-check). Our own
+# script/check: run all checks (test, test-verify-build, lint, fmt-check).
-# extension to scripts-to-rule-them-all. Must not modify any files.
+# Our own extension to scripts-to-rule-them-all. Must not modify any files.
 set -eu
 
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
 
 main() {
     "$SCRIPT_DIR/test"
+    "$SCRIPT_DIR/test-verify-build"
     "$SCRIPT_DIR/lint"
     "$SCRIPT_DIR/fmt-check"
 }

script/test-e2e-firefox

@@ -0,0 +1,63 @@
+#!/bin/sh
+# script/test-e2e-firefox: build the extension and drive the real popup in
+# a real Firefox inside a pinned container. The Firefox counterpart to
+# script/test-e2e. Our own extension to scripts-to-rule-them-all.
+#
+# Deliberately NOT called by script/check or script/test, for the same
+# reason as the Chrome suite: REPO_POLICIES.md caps make test at 20 seconds
+# and a browser suite does not fit.
+#
+# Unlike script/test-e2e this builds its image locally, because no
+# published image carries both a pinned Firefox and a matching geckodriver.
+# All three external artifacts are pinned by digest inside the Dockerfile;
+# see tests/e2e/firefox/Dockerfile.
+set -eu
+
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
+ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
+
+IMAGE="$("$SCRIPT_DIR/projectname")-e2e-firefox"
+
+main() {
+    cd "$ROOT"
+
+    if ! command -v docker >/dev/null 2>&1; then
+        echo "test-e2e-firefox: docker is required to run the e2e suite" >&2
+        exit 1
+    fi
+
+    echo "Building extension for e2e..."
+    yarn run build 2>&1
+
+    # The build context is tests/e2e/firefox/ and holds nothing but the
+    # Dockerfile: the harness itself arrives over the bind mount below, so
+    # editing it never invalidates an image layer.
+    echo "Building the pinned Firefox e2e image..."
+    docker build -t "$IMAGE" "$ROOT/tests/e2e/firefox"
+
+    echo "Running the Firefox e2e suite..."
+    # --shm-size=1g: Firefox needs more than the default 64MB /dev/shm.
+    # --network none: the suite stubs nothing, so this is what keeps the
+    #   run offline and deterministic. The extension swallows its own
+    #   fetch failures, so the popup flows work unchanged; see the
+    #   network note in README.md. Weaker than the Chrome suite's
+    #   fixture interception, and honestly so — it proves no request
+    #   escaped, but it cannot report which ones were attempted.
+    # --user: keep files the suite touches owned by the caller, not root.
+    # HOME=/tmp: the mapped uid has no home directory in the image.
+    #
+    # No --privileged. Firefox's sandbox logs
+    # "CanCreateUserNamespace() clone() failure: EPERM" on startup here;
+    # it is cosmetic and headless Firefox runs fine without it.
+    docker run --rm \
+        --shm-size=1g \
+        --network none \
+        --user "$(id -u):$(id -g)" \
+        -e HOME=/tmp \
+        -v "$ROOT:/work" \
+        -w /work \
+        "$IMAGE" \
+        node tests/e2e/firefox/run.js dist/firefox
+}
+
+main "$@"

script/test-verify-build

@@ -0,0 +1,444 @@
+#!/bin/sh
+# script/test-verify-build: exercise every failure mode of
+# script/verify-build. Our own extension to scripts-to-rule-them-all, run
+# from script/check so make check covers it.
+#
+# Why this exists: verify-build is the build-integrity guard, and three
+# separate reviews of it each found a fresh vacuous pass — the grep exit-2
+# conflation, the discarded find status, the line-delimited walk. Every one
+# was caught by someone building a tree by hand, because nothing in make check
+# could catch it. This is that hand battery, committed and automated.
+#
+# Each case asserts the exit status AND a substring of the message. A guard
+# that fails for the wrong reason (right status, different fault) is itself a
+# defect, so matching the status alone would not be a test of anything.
+#
+# The fixture is a temp tree containing script/verify-build as a SYMLINK to
+# the real script: verify-build takes its ROOT from dirname "$0"/.., so it
+# operates on the fixture's dist/ and never reads or writes the repo's build
+# output. The symlink rather than a copy is what makes a deliberate break in
+# the real script fail here.
+set -eu
+
+ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
+VERIFY_BUILD="$ROOT/script/verify-build"
+
+MARKER_ON="autistmask-build-debug=on"
+MARKER_OFF="autistmask-build-debug=off"
+
+NEWLINE='
+'
+
+PASSED=0
+FAILED=0
+SKIPPED=0
+SKIPPED_NAMES=""
+
+# The command prefix that runs the permission-dependent cases as a user who
+# is actually subject to file permissions, and whether those cases can run at
+# all. Both are decided by probe_permission_runner, never assumed.
+UNPRIV=""
+PERM_ENABLED=no
+PERM_HOW=""
+
+WORK=""
+
+cleanup() {
+    [ -n "$WORK" ] || return 0
+    # The cases chmod 000 files and directories on purpose.
+    chmod -R u+rwX "$WORK" 2>/dev/null || true
+    rm -rf "$WORK"
+}
+trap cleanup EXIT INT TERM
+
+WORK="$(mktemp -d "${TMPDIR:-/tmp}/autistmask-test-verify-build.XXXXXX")"
+FIXTURE="$WORK/fixture"
+
+# verify-build mktemps its dist/ listing under TMPDIR. Pointing that inside
+# our work dir keeps the run leaving no residue, and keeps it writable for the
+# unprivileged user the permission cases run as.
+TMPDIR="$WORK/tmp"
+export TMPDIR
+mkdir -p "$TMPDIR"
+chmod 1777 "$TMPDIR"
+chmod 755 "$WORK"
+
+# --- fixture ---------------------------------------------------------------
+
+# A stand-in for an emitted bundle: some text plus one marker literal, which
+# is all verify-build reads out of the real thing.
+write_bundle() {
+    printf 'var a=1;/* %s */\nvar b=2;\n' "$2" >"$1"
+}
+
+# A dist/ shaped like a real build: two listed bundles under different
+# browsers, an unlisted subtree to make unwalkable, and unlisted files that
+# carry no marker and must not be objected to.
+build_fixture() {
+    chmod -R u+rwX "$FIXTURE" 2>/dev/null || true
+    rm -rf "$FIXTURE"
+
+    mkdir -p "$FIXTURE/script"
+    ln -s "$VERIFY_BUILD" "$FIXTURE/script/verify-build"
+
+    mkdir -p "$FIXTURE/dist/chrome/src/popup" \
+        "$FIXTURE/dist/chrome/src/content" \
+        "$FIXTURE/dist/firefox/src/popup"
+
+    write_bundle "$FIXTURE/dist/chrome/src/popup/index.js" "$MARKER_OFF"
+    write_bundle "$FIXTURE/dist/firefox/src/popup/index.js" "$MARKER_OFF"
+    printf 'body{color:#000}\n' >"$FIXTURE/dist/styles.css"
+    printf 'var c=3;\n' >"$FIXTURE/dist/chrome/src/content/content.js"
+
+    {
+        echo "dist/chrome/src/popup/index.js"
+        echo "dist/firefox/src/popup/index.js"
+    } >"$FIXTURE/dist/constants-bundles.txt"
+
+    # Readable and traversable by the unprivileged user the permission cases
+    # run as, before those cases take that away again on purpose.
+    chmod -R a+rX "$FIXTURE"
+}
+
+# --- permission runner ------------------------------------------------------
+
+# Run a command through the current unprivileged runner. Unquoted on purpose:
+# UNPRIV is a command prefix that has to word-split.
+run_unpriv() {
+    # shellcheck disable=SC2086
+    $UNPRIV "$@"
+}
+
+# Decide whether the permission-dependent cases can run, and prove it rather
+# than assuming it.
+#
+# The problem: the CI image declares no USER, so CI runs as root, and root is
+# not subject to file permissions — chmod 000 stops neither find nor grep. A
+# permission case run as root passes vacuously, which is worse than no case at
+# all because it reads as coverage.
+#
+# So the runner is validated with two probes before any permission case is
+# counted:
+#
+#   - a mode-644 file MUST be readable through it. If not, the runner itself
+#     is broken (missing helper, no such user, sandbox), and every case run
+#     through it would fail for the wrong reason.
+#   - a mode-000 file MUST NOT be readable through it. If it is, permissions
+#     are not in force and the cases would pass without proving anything.
+#
+# Unprivileged: the runner is empty and both probes are about this process,
+# which is the honest answer. Root: setpriv and runuser are tried, both
+# present in the pinned CI base image. Only when no candidate passes both
+# probes are the cases skipped, and a skipped run says so unmistakably.
+probe_permission_runner() {
+    _probe="$WORK/probe"
+    mkdir -p "$_probe"
+    printf 'readable\n' >"$_probe/public"
+    printf 'secret\n' >"$_probe/private"
+    chmod 755 "$_probe"
+    chmod 644 "$_probe/public"
+    chmod 000 "$_probe/private"
+
+    if [ "$(id -u)" -eq 0 ]; then
+        _candidates="setpriv|setpriv --reuid=65534 --regid=65534 --clear-groups --
+runuser|runuser -u nobody --"
+    else
+        _candidates="direct|"
+    fi
+
+    _tried=""
+    _saved_ifs="$IFS"
+    IFS="$NEWLINE"
+    for _line in $_candidates; do
+        IFS="$_saved_ifs"
+        _label="${_line%%|*}"
+        _cmd="${_line#*|}"
+        _tried="${_tried:+$_tried, }$_label"
+
+        if [ -n "$_cmd" ]; then
+            _bin="${_cmd%% *}"
+            command -v "$_bin" >/dev/null 2>&1 || continue
+        fi
+
+        UNPRIV="$_cmd"
+        # Broken or unusable runner: the cases would fail for the wrong
+        # reason. Reaching the script under test is part of usable.
+        run_unpriv cat "$_probe/public" >/dev/null 2>&1 || continue
+        run_unpriv cat "$VERIFY_BUILD" >/dev/null 2>&1 || continue
+        # Permissions not in force through this runner: the cases would pass
+        # without testing anything.
+        if run_unpriv cat "$_probe/private" >/dev/null 2>&1; then
+            continue
+        fi
+
+        PERM_ENABLED=yes
+        PERM_HOW="$_label"
+        IFS="$_saved_ifs"
+        return 0
+    done
+    IFS="$_saved_ifs"
+
+    UNPRIV=""
+    PERM_ENABLED=no
+    PERM_HOW="$_tried"
+}
+
+# --- case runner ------------------------------------------------------------
+
+# check_case <name> <perm:yes|no> <mode:release|debug> <status> <text> <setup>
+#
+# Rebuilds the fixture, applies <setup> inside it, runs verify-build, and
+# requires both the exit status and the message. <perm> marks a case that only
+# means anything when file permissions are in force.
+check_case() {
+    _name="$1"
+    _perm="$2"
+    _mode="$3"
+    _want_status="$4"
+    _want_text="$5"
+    _setup="$6"
+
+    if [ "$_perm" = yes ] && [ "$PERM_ENABLED" != yes ]; then
+        SKIPPED=$((SKIPPED + 1))
+        SKIPPED_NAMES="$SKIPPED_NAMES##   - $_name$NEWLINE"
+        echo "  SKIP (permissions not in force): $_name"
+        return 0
+    fi
+
+    build_fixture
+    if ! (cd "$FIXTURE" && "$_setup") >/dev/null 2>&1; then
+        FAILED=$((FAILED + 1))
+        echo "  FAIL: $_name"
+        echo "    the case's own setup failed, so nothing was tested."
+        return 0
+    fi
+
+    if [ "$_mode" = debug ]; then
+        _debug=1
+    else
+        _debug=""
+    fi
+
+    # Exported rather than set as a command prefix: run_unpriv is a function,
+    # and an assignment prefixed to a function call is not portable.
+    AUTISTMASK_DEBUG="$_debug"
+    export AUTISTMASK_DEBUG
+
+    _status=0
+    if [ "$_perm" = yes ]; then
+        _out="$(run_unpriv "$FIXTURE/script/verify-build" 2>&1)" || _status=$?
+    else
+        _out="$("$FIXTURE/script/verify-build" 2>&1)" || _status=$?
+    fi
+
+    _ok=yes
+    _why=""
+
+    if [ "$_status" -ne "$_want_status" ]; then
+        _ok=no
+        _why="exit status $_status, wanted $_want_status"
+    fi
+
+    # Same discipline verify-build itself applies to grep: 0 and 1 are
+    # answers, anything else is not, and must not be read as "no match".
+    _g=0
+    printf '%s\n' "$_out" | grep -q -F -e "$_want_text" || _g=$?
+    case "$_g" in
+    0) ;;
+    1)
+        _ok=no
+        _why="${_why:+$_why; }message did not contain: $_want_text"
+        ;;
+    *)
+        _ok=no
+        _why="${_why:+$_why; }grep exited $_g matching the message, so the
+      message was never checked"
+        ;;
+    esac
+
+    if [ "$_ok" = yes ]; then
+        PASSED=$((PASSED + 1))
+        echo "  ok: $_name"
+        return 0
+    fi
+
+    FAILED=$((FAILED + 1))
+    echo "  FAIL: $_name"
+    echo "    $_why"
+    echo "    --- verify-build output ---"
+    printf '%s\n' "$_out" | sed 's/^/    /'
+    echo "    --- end output ---"
+}
+
+# --- cases ------------------------------------------------------------------
+#
+# Each runs with the fixture as its working directory.
+
+c_control() { :; }
+
+c_trailing_space() {
+    cp dist/chrome/src/popup/index.js "dist/chrome/src/popup/index.js "
+}
+
+c_embedded_newline() {
+    cp dist/chrome/src/popup/index.js "dist/chrome/src/popup/index.js$NEWLINE"
+}
+
+c_dist_symlink() {
+    mv dist dist.real
+    ln -s dist.real dist
+}
+
+c_unwalkable_subtree() { chmod 000 dist/chrome/src/content; }
+
+c_dangling_symlink() {
+    ln -s /nonexistent-target-for-test-verify-build dist/chrome/dangling.js
+}
+
+c_dir_symlink() { ln -s src dist/chrome/link-to-dir; }
+
+c_alias_symlink() { ln -s popup/index.js dist/chrome/src/aliased.js; }
+
+c_manifest_missing() { rm dist/constants-bundles.txt; }
+
+c_manifest_empty() { : >dist/constants-bundles.txt; }
+
+c_manifest_unreadable() { chmod 000 dist/constants-bundles.txt; }
+
+c_bundle_missing() { rm dist/chrome/src/popup/index.js; }
+
+c_bundle_empty() { : >dist/chrome/src/popup/index.js; }
+
+c_bundle_unreadable() { chmod 000 dist/chrome/src/popup/index.js; }
+
+c_unlisted_extension() {
+    cp dist/chrome/src/popup/index.js dist/chrome/src/popup/extra.mjs
+}
+
+c_no_marker() { printf 'var d=4;\n' >dist/chrome/src/popup/index.js; }
+
+c_both_markers() {
+    printf '/* %s */\n' "$MARKER_ON" >>dist/chrome/src/popup/index.js
+}
+
+run_cases() {
+    check_case "control: untouched dist passes" \
+        no release 0 "2 bundle(s) verified $MARKER_OFF" c_control
+
+    check_case "unlisted marker-carrying file, trailing space in name" \
+        no release 1 "carries a debug marker but is absent from" \
+        c_trailing_space
+
+    check_case "unlisted marker-carrying file, newline in name" \
+        no release 1 "carries a debug marker but is absent from" \
+        c_embedded_newline
+
+    check_case "dist/ replaced by a symlink" \
+        no release 1 "dist is a symlink, not a directory." c_dist_symlink
+
+    check_case "unwalkable subtree under dist/" \
+        yes release 1 "enumerating dist/, so part of the tree" \
+        c_unwalkable_subtree
+
+    check_case "dangling symlink under dist/" \
+        no release 1 \
+        "reading dist/chrome/dangling.js, so the file could not be" \
+        c_dangling_symlink
+
+    check_case "symlink to a directory under dist/" \
+        no release 1 \
+        "reading dist/chrome/link-to-dir, so the file could not be" \
+        c_dir_symlink
+
+    check_case "symlink to a listed bundle under an unlisted path" \
+        no release 1 \
+        "dist/chrome/src/aliased.js carries a debug marker but is absent" \
+        c_alias_symlink
+
+    check_case "manifest missing" \
+        no release 1 "dist/constants-bundles.txt is missing." \
+        c_manifest_missing
+
+    check_case "manifest empty" \
+        no release 1 "is empty, so no emitted bundle was found to contain" \
+        c_manifest_empty
+
+    check_case "manifest unreadable" \
+        yes release 1 "is not readable, so nothing was inspected." \
+        c_manifest_unreadable
+
+    check_case "listed bundle missing" \
+        no release 1 \
+        "lists dist/chrome/src/popup/index.js, which does not exist." \
+        c_bundle_missing
+
+    check_case "listed bundle empty" \
+        no release 1 "which is empty. An empty bundle" c_bundle_empty
+
+    check_case "listed bundle unreadable" \
+        yes release 1 \
+        "reading dist/chrome/src/popup/index.js, so the file could not be" \
+        c_bundle_unreadable
+
+    check_case "unlisted extension carrying a marker" \
+        no release 1 \
+        "dist/chrome/src/popup/extra.mjs carries a debug marker but is" \
+        c_unlisted_extension
+
+    check_case "listed bundle carries no marker" \
+        no release 1 "carries no debug marker, so its DEBUG state cannot be" \
+        c_no_marker
+
+    check_case "listed bundle carries both markers" \
+        no release 1 "carries both debug markers, so DEBUG was not resolved" \
+        c_both_markers
+
+    check_case "wrong marker for the requested mode" \
+        no debug 1 "is $MARKER_OFF but this build expects $MARKER_ON" \
+        c_control
+}
+
+# --- main --------------------------------------------------------------------
+
+main() {
+    cd "$ROOT"
+
+    [ -x "$VERIFY_BUILD" ] || {
+        echo "test-verify-build: $VERIFY_BUILD is missing or not executable" >&2
+        exit 1
+    }
+
+    echo "Testing script/verify-build failure modes..."
+    probe_permission_runner
+    if [ "$PERM_ENABLED" = yes ]; then
+        echo "  permission cases: enabled (runner: $PERM_HOW, proved against" \
+            "a mode-000 file)"
+    fi
+
+    run_cases
+
+    if [ "$FAILED" -ne 0 ]; then
+        echo "test-verify-build: $FAILED case(s) FAILED," \
+            "$PASSED passed, $SKIPPED skipped" >&2
+        exit 1
+    fi
+
+    if [ "$SKIPPED" -ne 0 ]; then
+        cat <<EOF
+################################################################################
+## WARNING: $SKIPPED PERMISSION CASE(S) DID NOT RUN, AND THIS RUN DOES NOT
+## PROVE THEM. This process is uid $(id -u), and no runner subject to file
+## permissions was available. Tried: $PERM_HOW.
+## Under root, chmod 000 stops neither find nor grep, so these cases would
+## have passed without testing anything. They were skipped, not counted:
+$SKIPPED_NAMES################################################################################
+EOF
+        echo "test-verify-build: $PASSED case(s) passed," \
+            "$SKIPPED SKIPPED AND NOT PROVEN (see the warning above)"
+        return 0
+    fi
+
+    echo "test-verify-build: $PASSED case(s) passed"
+}
+
+main "$@"

src/background/index.js

@@ -13,7 +13,16 @@ const {
 } = require("../shared/state");
 const { refreshBalances, getProvider } = require("../shared/balances");
 const { debugFetch, log } = require("../shared/log");
-const { verifySignedTx, verifySignature } = require("../shared/approvalVerify");
+const {
+    verifySignedTx,
+    verifySignature,
+    failureIsRetryable,
+    describeTxFailure,
+    TX_STAGE_SIGN,
+    TX_STAGE_VERIFY,
+    TX_STAGE_BROADCAST,
+    TX_STAGE_INFLIGHT,
+} = require("../shared/approvalVerify");
 const {
     isPhishingDomain,
     refreshPhishingListOnSchedule,
@@ -107,6 +116,55 @@ function resetPopupUrl() {
     }
 }
 
+// Settle a pending approval: hand `result` to the promise the requesting page
+// is waiting on and retire the approval. This is the ONLY place an approval is
+// resolved or removed — the popup closing, an active-address switch, a reject
+// from the popup and the attempt that signs and broadcasts all come through
+// here — because a settlement that bypasses the claim below is a fund-loss bug
+// and enumerating the call sites has repeatedly missed one.
+//
+// A claimed approval belongs to the attempt holding the claim, and only that
+// attempt may settle it. Anything else settling first would leave the attempt
+// running to completion against an already-settled promise: the transaction
+// reaches the chain while the page is told "User rejected the request", and the
+// user's natural response is to send it again at a fresh nonce.
+//
+// Returns false when the approval is gone or claimed by someone else, so the
+// caller can refuse instead of assuming it settled.
+function settleApproval(id, result, options) {
+    const approval = pendingApprovals[id];
+    if (!approval) return false;
+    const holdsClaim = !!(options && options.holdsClaim);
+    if (approval.attemptInFlight && !holdsClaim) return false;
+    delete pendingApprovals[id];
+    approval.resolve(result);
+    resetPopupUrl();
+    return true;
+}
+
+// Take exclusive hold of a pending approval for one attempt, or refuse.
+//
+// An approval that failed retryably has to stay in pendingApprovals, so its
+// presence cannot be the interlock against a second attempt; this flag is. It
+// is set synchronously, before the handler's first await, so a second response
+// carrying the same id — a reloaded approval window re-rendering a live
+// Approve button, a popup that emits the message twice — finds the attempt
+// already running instead of starting an independent verify and broadcast.
+// Without it one approval can put two transactions on the chain: with the
+// ordinary dApp approval shape the page fixes no nonce, so two artifacts
+// signed at different nonces both verify.
+function claimApproval(approval) {
+    if (approval.attemptInFlight) return false;
+    approval.attemptInFlight = true;
+    return true;
+}
+
+// Release an approval whose attempt failed in a way the user can retry.
+// Nothing was broadcast, so the next attempt may claim it.
+function releaseApproval(approval) {
+    approval.attemptInFlight = false;
+}
+
 // Open approval in a separate popup window.
 // This is the primary mechanism for tx/sign approvals (triggered programmatically,
 // not from a user gesture) and the fallback for site-connection approvals.
@@ -215,8 +273,7 @@ runtime.onConnect.addListener((port) => {
                     // Keep pending — user can reopen the toolbar popup
                     return;
                 }
-                approval.resolve({ approved: false, remember: false });
+                settleApproval(id, { approved: false, remember: false });
-                delete pendingApprovals[id];
             }
             resetPopupUrl();
         });
@@ -547,15 +604,21 @@ async function broadcastAccountsChanged() {
     for (const key of Object.keys(connectedSites)) {
         delete connectedSites[key];
     }
-    // Reject and close any pending approval popups so they don't hang
+    // Reject and close any pending approval popups so they don't hang. An
+    // approval an attempt has already claimed is left alone entirely: it is
+    // being signed and broadcast right now, and neither rejecting it to the
+    // page nor closing the window it is reporting into is survivable.
     for (const [id, approval] of Object.entries(pendingApprovals)) {
-        if (approval.type === "tx" || approval.type === "sign") {
+        const rejection =
-            approval.resolve({
+            approval.type === "tx" || approval.type === "sign"
-                error: { code: 4001, message: "User rejected the request." },
+                ? {
-            });
+                      error: {
-        } else {
+                          code: 4001,
-            approval.resolve({ approved: false, remember: false });
+                          message: "User rejected the request.",
-        }
+                      },
+                  }
+                : { approved: false, remember: false };
+        if (!settleApproval(id, rejection)) continue;
         if (approval.windowId) {
             windowsApi.remove(approval.windowId, () => {
                 if (runtime.lastError) {
@@ -563,7 +626,6 @@ async function broadcastAccountsChanged() {
                 }
             });
         }
-        delete pendingApprovals[id];
     }
     resetPopupUrl();
     const s = await getState();
@@ -679,23 +741,26 @@ if (runtime.onStartup) {
 }
 startBackgroundJobs();
 
-// When approval window is closed without a response, treat as rejection
+// When approval window is closed without a response, treat as rejection.
+// "Without a response" is the operative part: the popup stays open across the
+// verify and broadcast it is waiting on, so a user closing an apparently-hung
+// window is an ordinary event with an attempt already in flight behind it.
+// settleApproval() refuses those, which leaves the attempt to report its real
+// outcome to the page.
 if (windowsApi && windowsApi.onRemoved) {
     windowsApi.onRemoved.addListener((windowId) => {
         for (const [id, approval] of Object.entries(pendingApprovals)) {
-            if (approval.windowId === windowId) {
+            if (approval.windowId !== windowId) continue;
-                if (approval.type === "tx" || approval.type === "sign") {
+            const rejection =
-                    approval.resolve({
+                approval.type === "tx" || approval.type === "sign"
-                        error: {
+                    ? {
-                            code: 4001,
+                          error: {
-                            message: "User rejected the request.",
+                              code: 4001,
-                        },
+                              message: "User rejected the request.",
-                    });
+                          },
-                } else {
+                      }
-                    approval.resolve({ approved: false, remember: false });
+                    : { approved: false, remember: false };
-                }
+            settleApproval(id, rejection);
-                delete pendingApprovals[id];
-            }
         }
     });
 }
@@ -761,14 +826,10 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
     }
 
     if (msg.type === "AUTISTMASK_APPROVAL_RESPONSE") {
-        const approval = pendingApprovals[msg.id];
+        settleApproval(msg.id, {
-        if (approval) {
+            approved: msg.approved,
-            approval.resolve({
+            remember: msg.remember,
-                approved: msg.approved,
+        });
-                remember: msg.remember,
-            });
-            delete pendingApprovals[msg.id];
-        }
         resetPopupUrl();
         return false;
     }
@@ -776,21 +837,50 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
     if (msg.type === "AUTISTMASK_TX_RESPONSE") {
         const approval = pendingApprovals[msg.id];
         if (!approval) return false;
-        delete pendingApprovals[msg.id];
-        resetPopupUrl();
 
+        // A reject arriving while an attempt holds the approval is refused,
+        // not honoured: the attempt is on its way to broadcasting the
+        // transaction, and resolving 4001 here would tell the page the request
+        // was rejected while it goes out.
         if (!msg.approved) {
-            approval.resolve({
+            if (
-                error: { code: 4001, message: "User rejected the request." },
+                !settleApproval(msg.id, {
-            });
+                    error: {
+                        code: 4001,
+                        message: "User rejected the request.",
+                    },
+                })
+            ) {
+                sendResponse({
+                    error: "This transaction is already being sent.",
+                    retryable: false,
+                    stage: TX_STAGE_BROADCAST,
+                });
+                return false;
+            }
             return true;
         }
 
-        // The popup signs; it reports back here when it could not. Fail the
+        // The popup signs; it reports back here when it could not. Keep the
-        // request the same way this handler used to when it did the signing.
+        // approval so the user can correct the problem and try again with the
+        // transaction they already saw.
         if (msg.error) {
-            approval.resolve({ error: { message: msg.error } });
+            const outcome = describeTxFailure(TX_STAGE_SIGN, msg.error);
-            sendResponse({ error: msg.error });
+            sendResponse({
+                error: outcome.error,
+                retryable: outcome.retryable,
+                stage: TX_STAGE_SIGN,
+            });
+            return false;
+        }
+
+        // Exactly one broadcast per approval, whatever the popup sends.
+        if (!claimApproval(approval)) {
+            sendResponse({
+                error: "This transaction is already being sent.",
+                retryable: false,
+                stage: TX_STAGE_BROADCAST,
+            });
             return false;
         }
 
@@ -800,22 +890,63 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 const activeAddress = await getActiveAddress();
                 // The popup holds the secret, but the background stays the
                 // authority on what is broadcast: the raw transaction must be
-                // the approved one, signed by the approved address.
+                // the approved one, signed by the approved address, on the
+                // network that is selected.
                 verifySignedTx(
                     msg.rawSignedTx,
                     approval.txParams,
                     activeAddress,
+                    currentNetwork().chainId,
                 );
+            } catch (e) {
+                // A signed transaction that is not the approved one is not
+                // retried against that approval; it is refused outright.
+                // Anything else that failed before the check ran is the
+                // user's to retry.
+                const outcome = describeTxFailure(TX_STAGE_VERIFY, e);
+                if (outcome.spendApproval) {
+                    settleApproval(
+                        msg.id,
+                        { error: { message: outcome.error } },
+                        { holdsClaim: true },
+                    );
+                } else {
+                    releaseApproval(approval);
+                }
+                sendResponse({
+                    error: outcome.error,
+                    retryable: outcome.retryable,
+                    stage: TX_STAGE_VERIFY,
+                });
+                return;
+            }
+
+            try {
                 const provider = getProvider(state.rpcUrl);
                 const tx = await provider.broadcastTransaction(msg.rawSignedTx);
-                approval.resolve({ txHash: tx.hash });
+                settleApproval(
+                    msg.id,
+                    { txHash: tx.hash },
+                    { holdsClaim: true },
+                );
                 sendResponse({ txHash: tx.hash });
             } catch (e) {
-                const errMsg = e.shortMessage || e.message;
+                // Terminal, never retried: the node may have accepted the
-                approval.resolve({
+                // transaction and still failed to answer, and the popup's
-                    error: { message: errMsg },
+                // retry re-signs at a freshly fetched nonce rather than
+                // re-broadcasting these bytes. Retrying would send the
+                // approved transfer a second time.
+                const outcome = describeTxFailure(TX_STAGE_BROADCAST, e);
+                settleApproval(
+                    msg.id,
+                    { error: { message: outcome.error } },
+                    { holdsClaim: true },
+                );
+                sendResponse({
+                    error: outcome.error,
+                    retryable: outcome.retryable,
+                    stage: TX_STAGE_BROADCAST,
                 });
-                sendResponse({ error: errMsg });
             }
         })();
         return true;
@@ -824,21 +955,43 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
     if (msg.type === "AUTISTMASK_SIGN_RESPONSE") {
         const approval = pendingApprovals[msg.id];
         if (!approval) return false;
-        delete pendingApprovals[msg.id];
-        resetPopupUrl();
 
+        // Same as the transaction path: a reject cannot retire an approval an
+        // attempt already holds.
         if (!msg.approved) {
-            approval.resolve({
+            if (
-                error: { code: 4001, message: "User rejected the request." },
+                !settleApproval(msg.id, {
-            });
+                    error: {
+                        code: 4001,
+                        message: "User rejected the request.",
+                    },
+                })
+            ) {
+                sendResponse({
+                    error: "This request is already being signed.",
+                    retryable: false,
+                    stage: TX_STAGE_INFLIGHT,
+                });
+                return false;
+            }
             return true;
         }
 
-        // The popup signs; it reports back here when it could not. Fail the
+        // The popup signs; it reports back here when it could not. Keep the
-        // request the same way this handler used to when it did the signing.
+        // approval so the user can correct the problem and try again with the
+        // message they already saw.
         if (msg.error) {
-            approval.resolve({ error: { message: msg.error } });
+            sendResponse({ error: msg.error, retryable: true });
-            sendResponse({ error: msg.error });
+            return false;
+        }
+
+        // Exactly one signature handed back per approval.
+        if (!claimApproval(approval)) {
+            sendResponse({
+                error: "This request is already being signed.",
+                retryable: false,
+                stage: TX_STAGE_INFLIGHT,
+            });
             return false;
         }
 
@@ -851,14 +1004,21 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
                 // address.
                 const signature = msg.signature;
                 verifySignature(approval.signParams, signature, activeAddress);
-                approval.resolve({ signature });
+                settleApproval(msg.id, { signature }, { holdsClaim: true });
                 sendResponse({ signature });
             } catch (e) {
                 const errMsg = e.shortMessage || e.message;
-                approval.resolve({
+                const retryable = failureIsRetryable(e);
-                    error: { message: errMsg },
+                if (!retryable) {
-                });
+                    settleApproval(
-                sendResponse({ error: errMsg });
+                        msg.id,
+                        { error: { message: errMsg } },
+                        { holdsClaim: true },
+                    );
+                } else {
+                    releaseApproval(approval);
+                }
+                sendResponse({ error: errMsg, retryable });
             }
         })();
         return true;

src/popup/index.html

@@ -1142,6 +1142,62 @@
                 </button>
             </div>
 
+            <!-- ============ DELETE ADDRESS CONFIRM ============ -->
+            <div id="view-delete-address-confirm" class="view hidden">
+                <button
+                    id="btn-delete-address-back"
+                    class="border border-border px-2 py-1 hover:bg-fg hover:text-bg cursor-pointer mb-2"
+                >
+                    &lt; Back
+                </button>
+                <h2 class="font-bold mb-3">Remove Address</h2>
+                <p class="text-xs mb-2">
+                    You are about to remove
+                    <strong id="delete-address-label"></strong> from
+                    <strong id="delete-address-wallet-name"></strong>.
+                </p>
+                <div
+                    id="delete-address-value"
+                    class="text-xs mb-2 break-all min-h-[1rem]"
+                ></div>
+                <div
+                    class="text-xs mb-2 border border-border border-dashed p-2"
+                >
+                    This only stops this wallet from tracking the address.
+                    Nothing is destroyed and no key is deleted. Any funds at the
+                    address stay exactly where they are, and the address remains
+                    yours. Any site permissions granted to this address are
+                    forgotten.
+                </div>
+                <!-- Filled by src/popup/views/deleteAddress.js: the route
+                     back names the wallet's own kind of key material. -->
+                <div
+                    id="delete-address-recovery"
+                    class="text-xs mb-2 border border-border border-dashed p-2"
+                ></div>
+                <div
+                    id="delete-address-balance"
+                    class="text-xs mb-2 min-h-[1.25rem] pointer-events-none"
+                >
+                    &nbsp;
+                </div>
+                <p class="text-xs text-muted mb-3">
+                    A wallet always keeps at least one address. To remove the
+                    last one, delete the whole wallet from Settings instead.
+                </p>
+                <div
+                    id="delete-address-flash"
+                    class="text-xs text-red-500 mb-2 min-h-[1.25rem]"
+                    style="visibility: hidden"
+                ></div>
+                <button
+                    id="btn-delete-address-confirm"
+                    class="border border-border text-red-500 px-2 py-1 hover:bg-fg hover:text-bg cursor-pointer"
+                >
+                    Remove Address
+                </button>
+            </div>
+
             <!-- ============ SHOW RECOVERY PHRASE ============ -->
             <div id="view-show-phrase" class="view hidden">
                 <button

src/popup/index.js

@@ -33,6 +33,7 @@ const receive = require("./views/receive");
 const addToken = require("./views/addToken");
 const settings = require("./views/settings");
 const settingsAddToken = require("./views/settingsAddToken");
+const deleteAddress = require("./views/deleteAddress");
 const approval = require("./views/approval");
 
 function renderWalletList() {
@@ -101,6 +102,10 @@ const ctx = {
         pushCurrentView();
         settingsAddToken.show();
     },
+    showDeleteAddress: (walletIdx, addrIdx) => {
+        pushCurrentView();
+        deleteAddress.show(walletIdx, addrIdx);
+    },
 };
 
 function needsAddress(view) {
@@ -256,6 +261,7 @@ async function init() {
     addToken.init(ctx);
     settings.init(ctx);
     settingsAddToken.init(ctx);
+    deleteAddress.init(ctx);
 
     if (!state.hasWallet) {
         showView("welcome");

src/popup/views/approval.js

@@ -24,6 +24,7 @@ const { decryptWithPassword } = require("../../shared/vault");
 const { getSignerForAddress } = require("../../shared/wallet");
 const { walletDefect } = require("../../shared/walletDefects");
 const { getProvider } = require("../../shared/balances");
+const { describeSigningFailure } = require("../../shared/approvalVerify");
 const txStatus = require("./txStatus");
 const uniswap = require("../../shared/uniswap");
 const runtime =
@@ -589,10 +590,20 @@ function init(ctx) {
         runtime.sendMessage(payload, (response) => {
             if (response && response.txHash) {
                 txStatus.showWait(pendingTxDetails, response.txHash);
+                return;
+            }
+            // A retryable failure leaves the approval pending in the
+            // background, so stay on this screen with a live button rather
+            // than sending the user to a dead end.
+            const outcome = describeSigningFailure(
+                response,
+                "The transaction could not be sent.",
+            );
+            if (outcome.retryable) {
+                showError("approve-tx-error", outcome.message);
+                setTxButtonBusy(false);
             } else {
-                const msg =
+                txStatus.showError(pendingTxDetails, null, outcome.message);
-                    (response && response.error) || "Transaction failed.";
-                txStatus.showError(pendingTxDetails, null, msg);
             }
         });
     });
@@ -695,11 +706,18 @@ function init(ctx) {
         runtime.sendMessage(payload, (response) => {
             if (response && response.signature) {
                 window.close();
-            } else {
+                return;
-                const msg = (response && response.error) || "Signing failed.";
-                showError("approve-sign-error", msg);
-                setSignButtonBusy(false);
             }
+            // The button comes back only when the approval is still pending in
+            // the background; otherwise it stays disabled and the message says
+            // why, because a control that cannot succeed must not look like it
+            // can.
+            const outcome = describeSigningFailure(
+                response,
+                "The message could not be signed.",
+            );
+            showError("approve-sign-error", outcome.message);
+            if (outcome.retryable) setSignButtonBusy(false);
         });
     });
 

src/popup/views/deleteAddress.js

@@ -0,0 +1,176 @@
+// Confirmation screen for removing one address from a wallet that derives
+// its addresses from an extended key.
+//
+// No password is asked for, unlike delete-wallet. A password gates the
+// disclosure or destruction of a secret, and this does neither: the address
+// is derived from key material the wallet still holds, so removing it only
+// stops the wallet tracking it. An explicit confirmation screen is the
+// proportionate treatment.
+
+const {
+    $,
+    showView,
+    showFlash,
+    goBack,
+    renderAddressHtml,
+    attachCopyHandlers,
+    addressHoldsFunds,
+    balanceLinesForAddress,
+} = require("./helpers");
+const { formatUsd, getAddressValueUsd } = require("../../shared/prices");
+const { walletHasRecoveryPhrase } = require("../../shared/wallet");
+const { state, saveState } = require("../../shared/state");
+const {
+    canRemoveAddress,
+    removeAddressFromState,
+    broadcastActiveChanged,
+} = require("../../shared/walletDelete");
+
+// The wallet and address indices this screen is confirming, or null when it
+// is not confirming anything.
+let target = null;
+let ctx = null;
+
+function setFlash(msg) {
+    const el = $("delete-address-flash");
+    el.textContent = msg;
+    el.style.visibility = msg ? "visible" : "hidden";
+}
+
+// What it actually takes to get the address back, which is not what the
+// screen used to claim.
+//
+// Neither obvious route works: "+" derives the next unused index, because
+// wallet.nextIndex is a high-water mark and is deliberately not rewound; and
+// re-importing this wallet's key material is refused as a duplicate by
+// findWalletByXpub() for as long as the wallet is here. What remains is to
+// delete the whole wallet in Settings — which asks for the password and
+// destroys the stored secret — and import again, after which
+// scanForAddresses() rediscovers the address only if it has on-chain
+// activity. An address that was never used is not found by that scan, and
+// the copy must not imply otherwise.
+//
+// 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";
+    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 " +
+        "importing this " +
+        secret +
+        " again is refused while this wallet is still here. The way back is " +
+        "to delete the whole wallet in Settings, which asks for your " +
+        "password and destroys the stored " +
+        secret +
+        ", and then import that " +
+        secret +
+        " again. The scan that follows only finds addresses that have " +
+        "on-chain activity, so an address that has never been used is not " +
+        "found by it."
+    );
+}
+
+// The balance warning, or a blank line when the address holds nothing.
+//
+// A balance is a reason to be careful, not a reason to refuse: the funds are
+// at the address, not in this list, and stay there either way.
+//
+// "Holds" means ETH or any ERC-20 the wallet knows about — an address with no
+// ETH and a five-figure stablecoin position must not get the blank line on
+// the one screen whose job is to warn. The sentence names no figure of its
+// own: the rendered 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 lines below it carry the amounts, in the same format as Home and
+// AddressDetail, followed by the USD total when prices are known (null on
+// testnet and before the first price fetch, where the line is left off rather
+// than printed as $0.00).
+function balanceWarningHtml(addr) {
+    if (!addressHoldsFunds(addr)) return " ";
+    const usd = getAddressValueUsd(addr);
+    const total =
+        usd === null
+            ? ""
+            : `<div class="text-xs text-muted mt-1">Total: ${formatUsd(usd)}</div>`;
+    return (
+        `<p class="mb-1">This address holds a balance. Removing it does not ` +
+        `move or spend anything; the balance stays at the address.</p>` +
+        balanceLinesForAddress(addr, state.trackedTokens, false) +
+        total
+    );
+}
+
+function show(walletIdx, addrIdx) {
+    const wallet = state.wallets[walletIdx];
+    const addr = wallet && wallet.addresses[addrIdx];
+    if (!addr) return;
+    target = { walletIdx, addrIdx };
+
+    $("delete-address-label").textContent = "Address " + (addrIdx + 1);
+    $("delete-address-wallet-name").textContent =
+        wallet.name || "Wallet " + (walletIdx + 1);
+
+    const value = $("delete-address-value");
+    value.innerHTML = renderAddressHtml(addr.address, {
+        ensName: addr.ensName,
+    });
+    attachCopyHandlers(value);
+
+    $("delete-address-recovery").textContent = recoveryPathText(wallet);
+    $("delete-address-balance").innerHTML = balanceWarningHtml(addr);
+
+    setFlash("");
+    showView("delete-address-confirm");
+}
+
+function init(_ctx) {
+    ctx = _ctx;
+
+    $("btn-delete-address-back").addEventListener("click", () => {
+        target = null;
+        goBack();
+    });
+
+    $("btn-delete-address-confirm").addEventListener("click", async () => {
+        if (target === null) {
+            setFlash("No address is selected for removal.");
+            return;
+        }
+
+        const { walletIdx, addrIdx } = target;
+        if (!canRemoveAddress(state.wallets[walletIdx])) {
+            setFlash(
+                "This address cannot be removed, because a wallet always " +
+                    "keeps at least one address.",
+            );
+            return;
+        }
+
+        const { removed, activeAddressChanged } = removeAddressFromState(
+            state,
+            walletIdx,
+            addrIdx,
+        );
+        if (!removed) {
+            setFlash("This address could not be removed.");
+            return;
+        }
+
+        target = null;
+        // Save before broadcasting: the background reads the active address
+        // back out of storage to build accountsChanged.
+        await saveState();
+        if (activeAddressChanged) broadcastActiveChanged();
+
+        ctx.renderWalletList();
+        goBack();
+        showFlash("Address removed.");
+    });
+}
+
+// recoveryPathText and balanceWarningHtml are exported so the two pieces of
+// copy that carry the screen's substance can be tested without a DOM; show()
+// is a one-line assignment for each.
+module.exports = { init, show, recoveryPathText, balanceWarningHtml };

src/popup/views/helpers.js

@@ -25,6 +25,7 @@ const VIEWS = [
     "add-token",
     "settings",
     "delete-wallet-confirm",
+    "delete-address-confirm",
     "settings-addtoken",
     "transaction",
     "approve-site",
@@ -217,6 +218,20 @@ function balanceLinesForAddress(addr, trackedTokens, showZero) {
     return html;
 }
 
+// Whether an address holds anything at all: ETH or any ERC-20 the wallet
+// knows about. Deliberately unrounded — the rendered lines round to four
+// decimals, so a dust balance displays as 0.0000 while still being real
+// money at a real address. Callers that warn about holdings must ask this,
+// not the rendered figure.
+function addressHoldsFunds(addr) {
+    if (!addr) return false;
+    if (parseFloat(addr.balance || "0") > 0) return true;
+    for (const t of addr.tokenBalances || []) {
+        if (parseFloat(t.balance || "0") > 0) return true;
+    }
+    return false;
+}
+
 // Truncate the middle of a string, replacing removed characters with "…".
 // Safety: refuses to truncate more than 10 characters, which is the maximum
 // that still prevents address spoofing attacks (see Display Consistency in
@@ -463,6 +478,7 @@ module.exports = {
     flashCopyFeedback,
     balanceLine,
     balanceLinesForAddress,
+    addressHoldsFunds,
     addressColor,
     addressDotHtml,
     escapeHtml,

src/popup/views/home.js

@@ -21,6 +21,7 @@ const {
     resetSendValidation,
 } = require("./send");
 const { deriveAddressFromXpub } = require("../../shared/wallet");
+const { canRemoveAddress } = require("../../shared/walletDelete");
 const {
     walletDefect,
     walletDefectHtml,
@@ -240,6 +241,12 @@ function walletListHtml() {
             html += `<div class="address-row py-1 border-b border-border-light cursor-pointer hover:bg-hover" data-wallet="${wi}" data-address="${ai}">`;
             const isActive = state.activeAddress === addr.address;
             const infoBtn = `<span class="btn-addr-info text-xs cursor-pointer border border-border hover:bg-fg hover:text-bg" style="padding:0" data-wallet="${wi}" data-address="${ai}">[info]</span>`;
+            // Only where a wallet can spare the address: a wallet holding a
+            // single address has no remove control, because its last address
+            // is never removable.
+            const removeBtn = canRemoveAddress(wallet)
+                ? `<span class="btn-remove-address text-xs cursor-pointer border border-border hover:bg-fg hover:text-bg ml-1" style="padding:0" data-wallet="${wi}" data-address="${ai}" title="Remove this address from the wallet">[x]</span>`
+                : "";
             const dot = addressDotHtml(addr.address);
             const titleBold = isActive ? "font-bold" : "";
             html += `<div class="text-xs ${titleBold}">Address ${ai + 1}</div>`;
@@ -248,7 +255,7 @@ function walletListHtml() {
             }
             html += `<div class="flex text-xs items-center justify-between">`;
             html += `<span class="flex items-center break-all">${addr.ensName ? "" : dot}${addr.address}</span>`;
-            html += `<span class="flex-shrink-0 ml-1">${infoBtn}</span>`;
+            html += `<span class="flex-shrink-0 ml-1">${infoBtn}${removeBtn}</span>`;
             html += `</div>`;
             const addrUsd = formatUsd(getAddressValueUsd(addr));
             html += `<div class="text-xs text-muted text-right min-h-[1rem]">${addrUsd || "&nbsp;"}</div>`;
@@ -304,6 +311,16 @@ function render(ctx) {
         });
     });
 
+    container.querySelectorAll(".btn-remove-address").forEach((btn) => {
+        btn.addEventListener("click", (e) => {
+            e.stopPropagation();
+            ctx.showDeleteAddress(
+                parseInt(btn.dataset.wallet, 10),
+                parseInt(btn.dataset.address, 10),
+            );
+        });
+    });
+
     container.querySelectorAll(".btn-add-address").forEach((btn) => {
         btn.addEventListener("click", async (e) => {
             e.stopPropagation();

src/popup/views/send.js

@@ -12,8 +12,9 @@ const {
 const { state, currentAddress } = require("../../shared/state");
 let ctx;
 const { getProvider } = require("../../shared/balances");
-const { KNOWN_SYMBOLS, resolveSymbol } = require("../../shared/tokenList");
+const { resolveSymbol } = require("../../shared/tokenList");
 const { isLowHolderCount } = require("../../shared/holders");
+const { isSpoofedSymbol } = require("../../shared/symbolSpoof");
 const { getAddress } = require("ethers");
 
 const ZERO_ADDRESS = "0x0000000000000000000000000000000000000000";
@@ -116,14 +117,6 @@ function updateToValidation() {
     }
 }
 
-function isSpoofedToken(t) {
-    const upper = (t.symbol || "").toUpperCase();
-    if (!KNOWN_SYMBOLS.has(upper)) return false;
-    const legit = KNOWN_SYMBOLS.get(upper);
-    if (legit === null) return true;
-    return t.address.toLowerCase() !== legit;
-}
-
 function renderSendTokenSelect(addr) {
     const sel = $("send-token");
     sel.innerHTML = '<option value="ETH">ETH</option>';
@@ -131,7 +124,7 @@ function renderSendTokenSelect(addr) {
         (state.fraudContracts || []).map((a) => a.toLowerCase()),
     );
     for (const t of addr.tokenBalances || []) {
-        if (isSpoofedToken(t)) continue;
+        if (isSpoofedSymbol(t.symbol, t.address)) continue;
         if (fraudSet.has(t.address.toLowerCase())) continue;
         // An unknown holder count does not withhold a token the user holds:
         // only a count the explorer actually reported as below the threshold

src/shared/approvalVerify.js

@@ -7,17 +7,144 @@
 // the signer from the artifact and checks it against the approval it is
 // holding before acting on it. All recovery is delegated to ethers.
 //
+// The check is an allowlist, in both directions, because a denylist cannot be
+// correct against a transaction format that keeps gaining fields:
+//
+//  - only transaction types 0, 1 and 2 are accepted. Every later EIP-2718 type
+//    adds a field with consequences of its own — EIP-7702's authorizationList
+//    rewrites the code at the signer's own account, EIP-4844's blob
+//    commitments carry a separate fee — and a check that enumerates the fields
+//    it refuses admits every one of them by default.
+//  - after the per-field comparisons, the artifact is rebuilt from those
+//    checked fields and nothing else, and the two are compared byte for byte.
+//    Anything the artifact carries that this module does not name is absent
+//    from the rebuild and changes the bytes, so the final assertion is that
+//    the artifact *is* the approved transaction, not merely that it is not one
+//    of the tampered shapes that were thought of.
+//  - every comparison runs against the decode, but the string handed to
+//    broadcastTransaction() is the artifact. So the artifact is also required
+//    to be the canonical re-encoding of its own decode, which is what makes
+//    the checked transaction and the broadcast bytes the same object rather
+//    than two things that merely decode alike.
+//
+// Every consequential field is compared, and a mismatch is a refusal to act,
+// never a warning: what the user approved is what gets broadcast, or nothing
+// does.
+//
+// Fields the approval does not carry are not treated as zero. The popup
+// populates nonce, gas limit, fee and chain id through populateTransaction()
+// when the requesting page did not fix them, so there is no approved value to
+// compare against; treating absent as zero would refuse every legitimate
+// transaction. Those fields are instead held to the absolute ceilings below,
+// and the chain id is always checked against the selected network rather than
+// against the approval alone, which is what makes a cross-chain replay
+// impossible.
+//
 // Every failure message is a full sentence, because these strings are shown to
 // the user and returned to the dApp.
 
 const {
     Transaction,
+    accessListify,
     getAddress,
     getBytes,
     verifyMessage,
     verifyTypedData,
 } = require("ethers");
 
+// The only transaction types this wallet signs: legacy, EIP-2930 and
+// EIP-1559. populateTransaction() produces nothing else, so nothing else can
+// be an artifact of an approval this wallet raised.
+const ALLOWED_TX_TYPES = [0, 1, 2];
+
+// The serialized fields of each allowed type, which is also the complete set
+// of fields the checks below compare or bound. The artifact is rebuilt from
+// exactly these at the end of verification and compared byte for byte, so a
+// field outside this table cannot ride along unexamined.
+const SERIALIZED_FIELDS = {
+    0: ["chainId", "nonce", "gasPrice", "gasLimit", "to", "value", "data"],
+    1: [
+        "chainId",
+        "nonce",
+        "gasPrice",
+        "gasLimit",
+        "to",
+        "value",
+        "data",
+        "accessList",
+    ],
+    2: [
+        "chainId",
+        "nonce",
+        "maxPriorityFeePerGas",
+        "maxFeePerGas",
+        "gasLimit",
+        "to",
+        "value",
+        "data",
+        "accessList",
+    ],
+};
+
+// Fields no allowed type may carry. The type allowlist already excludes every
+// type that defines them, and the structural check at the end of verification
+// would catch them anyway; they are named here so that an artifact carrying
+// one is refused with a message that says what it was.
+const FORBIDDEN_FIELDS = [
+    {
+        key: "authorizationList",
+        message:
+            "The signed transaction would hand the signing account over to another contract, which was not approved.",
+    },
+    {
+        key: "blobVersionedHashes",
+        message:
+            "The signed transaction carries blob commitments, which were not approved.",
+    },
+    {
+        key: "blobs",
+        message:
+            "The signed transaction carries blobs, which were not approved.",
+    },
+    {
+        key: "maxFeePerBlobGas",
+        message:
+            "The signed transaction carries a blob gas fee, which was not approved.",
+    },
+];
+
+// Above the block gas limit of every supported network (see networks.js), so
+// no transaction that could ever be included is refused by it.
+const MAX_GAS_LIMIT = 100000000n;
+
+// 100,000 gwei per gas: orders of magnitude above the highest fee either
+// supported network has produced, and low enough to catch a fee that would
+// hand the validator the balance.
+const MAX_FEE_PER_GAS = 100000000000000n;
+
+// A refusal to act on an artifact: it is not the thing that was approved, so
+// the approval it was offered against is spent and must not be retried. Every
+// throw in this module is one of these; the background distinguishes them from
+// transient failures (a busy node, a failed broadcast), which leave the
+// approval standing so the user can try again.
+class ApprovalMismatchError extends Error {
+    constructor(message) {
+        super(message);
+        this.name = "ApprovalMismatchError";
+        this.approvalMismatch = true;
+    }
+}
+
+function refuse(message) {
+    return new ApprovalMismatchError(message);
+}
+
+// Whether a signing failure leaves the approval usable. Anything that is not a
+// mismatch is the user's to correct and retry.
+function failureIsRetryable(err) {
+    return !(err && err.approvalMismatch === true);
+}
+
 // Case-insensitive address comparison that tolerates absent values on either
 // side. Two absent addresses compare equal (contract creation has no `to`).
 function sameAddress(a, b) {
@@ -31,11 +158,64 @@ function sameAddress(a, b) {
     }
 }
 
+// Whether the approval fixed a value for a field at all.
+function present(v) {
+    return v !== null && v !== undefined && v !== "";
+}
+
+// Whether a field carries anything at all. An empty array is nothing: ethers
+// reports an absent access list on a type 2 transaction as `[]`.
+function carriesValue(v) {
+    if (!present(v)) return false;
+    if (Array.isArray(v)) return v.length > 0;
+    return true;
+}
+
+// Normalize a quantity that must be present, refusing anything that is not a
+// number: an approval carrying junk in a fee field cannot be compared, and an
+// uncomparable field is a refusal rather than a pass.
+function normalizeQuantity(v, label) {
+    try {
+        return BigInt(v);
+    } catch {
+        throw refuse(
+            "The approved " +
+                label +
+                " is not a number, so it cannot be" +
+                " compared with the signed transaction.",
+        );
+    }
+}
+
 // Normalize a transaction value (hex string, decimal string, number or
-// bigint) to a bigint. An absent value is zero, matching ethers.
+// bigint) to a bigint. An absent value is zero, matching ethers. The value is
+// page-controlled, so it goes through the same refusal as every other
+// quantity rather than throwing a raw BigInt conversion error.
 function normalizeValue(v) {
-    if (v === null || v === undefined || v === "") return 0n;
+    if (!present(v)) return 0n;
-    return BigInt(v);
+    return normalizeQuantity(v, "value");
+}
+
+// Normalize an access list to a comparable string. An absent or empty list is
+// the empty string, so absent and `[]` are the same thing.
+function normalizeAccessList(v) {
+    if (!carriesValue(v)) return "";
+    let list;
+    try {
+        list = accessListify(v);
+    } catch {
+        throw refuse(
+            "The approved access list is not a valid access list, so it cannot be compared with the signed transaction.",
+        );
+    }
+    return list
+        .map(
+            (entry) =>
+                String(entry.address).toLowerCase() +
+                ":" +
+                entry.storageKeys.map((k) => String(k).toLowerCase()).join(","),
+        )
+        .join(";");
 }
 
 // Normalize call data to a lowercase hex string. Absent data is "0x".
@@ -44,44 +224,216 @@ function normalizeData(v) {
     return String(v).toLowerCase();
 }
 
+// Quantity fields the requesting page may fix in the approval. Each is
+// compared exactly when the approval carries it, and left to the ceilings
+// above when it does not.
+const APPROVED_QUANTITIES = [
+    {
+        key: "nonce",
+        label: "nonce",
+        message: "The signed transaction does not carry the approved nonce.",
+    },
+    {
+        key: "gasLimit",
+        label: "gas limit",
+        message:
+            "The signed transaction does not carry the approved gas limit.",
+    },
+    {
+        key: "gasPrice",
+        label: "gas price",
+        message:
+            "The signed transaction does not carry the approved gas price.",
+    },
+    {
+        key: "maxFeePerGas",
+        label: "maximum fee per gas",
+        message:
+            "The signed transaction does not carry the approved maximum fee per gas.",
+    },
+    {
+        key: "maxPriorityFeePerGas",
+        label: "maximum priority fee per gas",
+        message:
+            "The signed transaction does not carry the approved maximum priority fee per gas.",
+    },
+];
+
+// Refuse a field only a transaction type this wallet does not sign can carry.
+// The type allowlist keeps these unreachable in production, which is exactly
+// what they are for; it also means nothing else exercises them, so this is
+// exported and tested on its own rather than left to be believed.
+function assertNoForbiddenFields(parsed) {
+    for (const field of FORBIDDEN_FIELDS) {
+        if (carriesValue(parsed[field.key])) throw refuse(field.message);
+    }
+}
+
+// Closing structural check. Rebuild the transaction from the fields the
+// comparisons cover, and nothing else, then compare the unsigned bytes. Every
+// field carried by the artifact but absent from the rebuild changes the
+// serialization, so this refuses anything this module does not account for —
+// including a field a future ethers learns to parse onto an allowed type —
+// instead of waving it through by not naming it. Also exported for its own
+// test: nothing reachable today can make the bytes differ.
+function assertNothingUnchecked(parsed) {
+    let rebuilt;
+    try {
+        const fields = { type: parsed.type };
+        for (const key of SERIALIZED_FIELDS[parsed.type]) {
+            fields[key] = parsed[key];
+        }
+        rebuilt = Transaction.from(fields);
+    } catch {
+        throw refuse(
+            "The signed transaction could not be rebuilt from the fields that were checked, so it cannot be shown to be the approved transaction.",
+        );
+    }
+    if (rebuilt.unsignedSerialized !== parsed.unsignedSerialized) {
+        throw refuse(
+            "The signed transaction carries data beyond the fields that were checked against the approval.",
+        );
+    }
+}
+
+// The other half of the closing check, and the one that makes it bind on the
+// bytes that actually leave: every comparison above runs against the decode,
+// so on its own the rebuild proves only that the transaction ethers understood
+// is the approved one. What the background hands to broadcastTransaction() is
+// the artifact string itself. Requiring the artifact to be exactly the
+// canonical re-encoding of its own decode closes the gap between the two —
+// no encoding the decoder normalizes away (a leading zero byte on an RLP
+// quantity, say) can differ from what was checked. Hex case is not part of the
+// encoding, so only that is normalized before comparing.
+function assertCanonicalBytes(parsed, rawSignedTx) {
+    if (parsed.serialized !== String(rawSignedTx).toLowerCase()) {
+        throw refuse(
+            "The signed transaction is not encoded canonically, so the bytes that would be broadcast are not the bytes that were checked.",
+        );
+    }
+}
+
 // Assert that a raw signed transaction is the transaction the user approved,
-// signed by the address the approval was raised for. Returns the parsed
+// signed by the address the approval was raised for, on the network that is
-// ethers Transaction on success, throws otherwise.
+// selected. Returns the parsed ethers Transaction on success, throws
-function verifySignedTx(rawSignedTx, txParams, expectedFrom) {
+// otherwise.
+function verifySignedTx(rawSignedTx, txParams, expectedFrom, selectedChainId) {
     if (typeof rawSignedTx !== "string" || !rawSignedTx.startsWith("0x")) {
-        throw new Error("The signed transaction is missing or malformed.");
+        throw refuse("The signed transaction is missing or malformed.");
     }
 
     let parsed;
     try {
         parsed = Transaction.from(rawSignedTx);
     } catch {
-        throw new Error("The signed transaction could not be decoded.");
+        throw refuse("The signed transaction could not be decoded.");
     }
 
     if (!parsed.from) {
-        throw new Error("The signed transaction carries no valid signature.");
+        throw refuse("The signed transaction carries no valid signature.");
     }
     if (!sameAddress(parsed.from, expectedFrom)) {
-        throw new Error(
+        throw refuse(
             "The signed transaction was signed by a different address than the one that was approved.",
         );
     }
+
+    // Before any field is looked at: the type decides which fields exist at
+    // all, so an unrecognised type is refused outright rather than compared
+    // field by field against an approval that cannot describe it.
+    if (!ALLOWED_TX_TYPES.includes(parsed.type)) {
+        throw refuse(
+            "The signed transaction is of a type this wallet does not sign, so what it would do beyond the approved transfer cannot be checked.",
+        );
+    }
+    assertNoForbiddenFields(parsed);
+
+    // The selected network, not the artifact, is the authority on which chain
+    // this may be broadcast to; without it nothing can be verified.
+    if (!present(selectedChainId)) {
+        throw refuse(
+            "The selected network is unknown, so the signed transaction cannot be checked against it.",
+        );
+    }
+    if (parsed.chainId !== normalizeQuantity(selectedChainId, "network")) {
+        throw refuse(
+            "The signed transaction is for a different network than the one that is selected.",
+        );
+    }
+    if (
+        present(txParams.chainId) &&
+        parsed.chainId !== normalizeQuantity(txParams.chainId, "network")
+    ) {
+        throw refuse(
+            "The signed transaction is for a different network than the one that was approved.",
+        );
+    }
+
     if (!sameAddress(parsed.to, txParams.to)) {
-        throw new Error(
+        throw refuse(
             "The signed transaction does not go to the approved recipient.",
         );
     }
     if (normalizeValue(parsed.value) !== normalizeValue(txParams.value)) {
-        throw new Error(
+        throw refuse(
             "The signed transaction does not carry the approved value.",
         );
     }
     if (normalizeData(parsed.data) !== normalizeData(txParams.data)) {
-        throw new Error(
+        throw refuse(
             "The signed transaction does not carry the approved call data.",
         );
     }
+    if (
+        normalizeAccessList(parsed.accessList) !==
+        normalizeAccessList(txParams.accessList)
+    ) {
+        throw refuse(
+            "The signed transaction does not carry the approved access list.",
+        );
+    }
+
+    // An approval that fixed EIP-1559 fees must not be signed as a legacy
+    // transaction, and vice versa: the fee the user agreed to is only
+    // meaningful under the mechanism it was quoted in.
+    const approvedEip1559 =
+        present(txParams.maxFeePerGas) ||
+        present(txParams.maxPriorityFeePerGas);
+    const approvedLegacy = present(txParams.gasPrice);
+    const signedEip1559 = parsed.type === 2;
+    if (
+        (approvedEip1559 && !signedEip1559) ||
+        (approvedLegacy && signedEip1559)
+    ) {
+        throw refuse(
+            "The signed transaction does not use the approved fee mechanism.",
+        );
+    }
+
+    for (const field of APPROVED_QUANTITIES) {
+        if (!present(txParams[field.key])) continue;
+        const approved = normalizeQuantity(txParams[field.key], field.label);
+        if (normalizeQuantity(parsed[field.key], field.label) !== approved) {
+            throw refuse(field.message);
+        }
+    }
+
+    if (parsed.gasLimit > MAX_GAS_LIMIT) {
+        throw refuse(
+            "The signed transaction sets a gas limit no network this wallet supports can accept.",
+        );
+    }
+    for (const key of ["gasPrice", "maxFeePerGas", "maxPriorityFeePerGas"]) {
+        const fee = parsed[key];
+        if (fee !== null && fee !== undefined && fee > MAX_FEE_PER_GAS) {
+            throw refuse(
+                "The signed transaction sets a fee per gas far above any plausible value.",
+            );
+        }
+    }
+
+    assertNothingUnchecked(parsed);
+    assertCanonicalBytes(parsed, rawSignedTx);
 
     return parsed;
 }
@@ -91,7 +443,7 @@ function verifySignedTx(rawSignedTx, txParams, expectedFrom) {
 // address on success, throws otherwise.
 function verifySignature(signParams, signature, expectedFrom) {
     if (typeof signature !== "string" || !signature.startsWith("0x")) {
-        throw new Error("The signature is missing or malformed.");
+        throw refuse("The signature is missing or malformed.");
     }
 
     let recovered;
@@ -109,11 +461,11 @@ function verifySignature(signParams, signature, expectedFrom) {
             recovered = verifyTypedData(domain, types, message, signature);
         }
     } catch {
-        throw new Error("The signature could not be verified.");
+        throw refuse("The signature could not be verified.");
     }
 
     if (!sameAddress(recovered, expectedFrom)) {
-        throw new Error(
+        throw refuse(
             "The signature was produced by a different address than the one that was approved.",
         );
     }
@@ -121,4 +473,98 @@ function verifySignature(signParams, signature, expectedFrom) {
     return recovered;
 }
 
-module.exports = { verifySignedTx, verifySignature, sameAddress };
+// The stage a transaction approval failed at. Which stage it is decides
+// whether the approval survives the failure.
+const TX_STAGE_SIGN = "sign";
+const TX_STAGE_VERIFY = "verify";
+const TX_STAGE_BROADCAST = "broadcast";
+// Not a failure of this request at all: a second response arrived for an
+// approval an attempt already holds. The first attempt is still running and
+// may yet succeed, so the one thing the popup must not say is "start again
+// from the site".
+const TX_STAGE_INFLIGHT = "inflight";
+
+function errorText(err) {
+    if (typeof err === "string" && err !== "") return err;
+    if (err && (err.shortMessage || err.message)) {
+        return err.shortMessage || err.message;
+    }
+    return "The transaction could not be sent.";
+}
+
+// What the background does with a pending transaction approval after a failed
+// attempt: what it tells the popup, and whether the approval is spent
+// (resolved to the requesting page as an error and deleted) or left standing
+// so the user can try the transaction they already saw again.
+//
+//  - sign: the popup could not produce an artifact, almost always a wrong
+//    password. Nothing left the extension, so the approval stands.
+//  - verify: a mismatch is a refusal and spends the approval — an artifact
+//    that is not the approved transaction must never be retried against that
+//    approval. Anything else failed before the check ran and is retryable.
+//  - broadcast: always terminal. A broadcast that throws after the node
+//    accepted the transaction is routine (a timeout, a dropped response, a
+//    node answering "already known"), and the popup's retry does not
+//    re-broadcast these bytes — it re-runs populateTransaction() and signs
+//    again at a freshly fetched pending-tag nonce. Retrying would therefore
+//    put a second transaction on the chain for one approval.
+function describeTxFailure(stage, err) {
+    const error = errorText(err);
+    const retryable =
+        stage === TX_STAGE_SIGN ||
+        (stage === TX_STAGE_VERIFY && failureIsRetryable(err));
+    return { error, retryable, spendApproval: !retryable };
+}
+
+// What the popup shows and does after the background reports a failed signing
+// attempt. A retryable failure leaves the approval pending in the background,
+// so the button goes back to being usable; a refusal spent the approval, and
+// the popup says so rather than offering a button that cannot succeed.
+//
+// A failed broadcast gets its own wording: the transaction may already be on
+// the network, so telling the user to start again from the site is exactly the
+// wrong instruction.
+function describeSigningFailure(response, fallbackMessage) {
+    let message = (response && response.error) || fallbackMessage;
+    if (!/[.!?]$/.test(message)) message += ".";
+    const retryable = !!(response && response.retryable);
+    const stage = response && response.stage;
+    if (!retryable) {
+        if (stage === TX_STAGE_BROADCAST) {
+            message +=
+                " The transaction may still have reached the network." +
+                " Check the account before sending it again.";
+        } else if (stage === TX_STAGE_INFLIGHT) {
+            message +=
+                " The first attempt is still running and may still succeed." +
+                " Wait for it rather than starting again.";
+        } else {
+            message +=
+                " This request can no longer be signed. Please start it" +
+                " again from the site.";
+        }
+    }
+    return { message, retryable };
+}
+
+module.exports = {
+    verifySignedTx,
+    verifySignature,
+    assertNoForbiddenFields,
+    assertNothingUnchecked,
+    assertCanonicalBytes,
+    sameAddress,
+    failureIsRetryable,
+    describeTxFailure,
+    describeSigningFailure,
+    ApprovalMismatchError,
+    ALLOWED_TX_TYPES,
+    SERIALIZED_FIELDS,
+    FORBIDDEN_FIELDS,
+    TX_STAGE_SIGN,
+    TX_STAGE_VERIFY,
+    TX_STAGE_BROADCAST,
+    TX_STAGE_INFLIGHT,
+    MAX_GAS_LIMIT,
+    MAX_FEE_PER_GAS,
+};

src/shared/balances.js

@@ -11,8 +11,9 @@ const {
 const { ERC20_ABI } = require("./constants");
 const { log, debugFetch } = require("./log");
 const { deriveAddressFromXpub } = require("./wallet");
-const { KNOWN_SYMBOLS, TOKEN_BY_ADDRESS } = require("./tokenList");
+const { TOKEN_BY_ADDRESS } = require("./tokenList");
 const { LOW_HOLDER_THRESHOLD, parseHoldersCount } = require("./holders");
+const { isSpoofedSymbol } = require("./symbolSpoof");
 
 // Use a static network to skip auto-detection (which can fail and cause
 // "could not coalesce error" on some RPC endpoints like Cloudflare).
@@ -89,15 +90,11 @@ async function fetchTokenBalances(address, blockscoutUrl, trackedTokens) {
             // Skip spam tokens the user never asked to see
             if (!isKnown && !isTracked && !hasEnoughHolders) continue;
 
-            // Skip tokens spoofing a known symbol from a different address
+            // Skip tokens spoofing a known symbol from a different address.
-            const sym = (item.token.symbol || "").toUpperCase();
+            // Every row here is an ERC-20 the explorer reported, so it has a
-            const legitAddr = KNOWN_SYMBOLS.get(sym);
+            // contract address; the native ETH balance is fetched over RPC in
-            if (
+            // refreshBalances and never passes through this loop.
-                legitAddr !== undefined &&
+            if (isSpoofedSymbol(item.token.symbol, tokenAddr)) continue;
-                legitAddr !== null &&
-                tokenAddr !== legitAddr
-            )
-                continue;
 
             balances.push({
                 address: item.token.address_hash,

src/shared/symbolSpoof.js

@@ -0,0 +1,43 @@
+// The known-symbol spoof rule, in one place.
+//
+// A token that borrows a known symbol from a contract that is not the one
+// that symbol belongs to is a spoof, and the wallet hides it.  Three surfaces
+// ask that question — the transaction history, the Send token selector and
+// the balance list — and they must answer it identically: a token the history
+// calls fake while the balance list lists it as a holding is worse than
+// either verdict alone, because the balance list is where the user forms
+// their belief about what they own (issue #235).
+//
+// KNOWN_SYMBOLS maps a symbol to the lowercased contract address that may
+// bear it, or to null.  Null means the symbol belongs to the native asset,
+// which has no contract at all, so no contract may bear it and every one
+// that does is a spoof.  "ETH" is the only such entry today; the rule is
+// written so that a second one needs no change here or at any call site.
+
+const { KNOWN_SYMBOLS } = require("./tokenList");
+
+// Ethereum addresses are case-insensitive: EIP-55 mixed case is a checksum
+// over the address, not part of its identity.
+function normalizeAddress(addr) {
+    return (addr || "").toLowerCase();
+}
+
+// True when a token bearing `symbol` from contract `contractAddress` is
+// impersonating a known symbol.
+//
+// An empty contract address is the native asset, which is never a spoof:
+// this is what keeps the user's real ETH out of the rule, and it holds for
+// any symbol that becomes null-mapped later, not just for ETH.
+function isSpoofedSymbol(symbol, contractAddress) {
+    const contract = normalizeAddress(contractAddress);
+    if (!contract) return false;
+    const sym = (symbol || "").toUpperCase();
+    if (!KNOWN_SYMBOLS.has(sym)) return false;
+    const legit = KNOWN_SYMBOLS.get(sym);
+    if (legit === null) return true;
+    return contract !== normalizeAddress(legit);
+}
+
+module.exports = {
+    isSpoofedSymbol,
+};

src/shared/transactions.js

@@ -8,8 +8,9 @@
 
 const { formatEther, formatUnits } = require("ethers");
 const { log, debugFetch } = require("./log");
-const { KNOWN_SYMBOLS, TOKEN_BY_ADDRESS } = require("./tokenList");
+const { TOKEN_BY_ADDRESS } = require("./tokenList");
 const { parseHoldersCount, isLowHolderCount } = require("./holders");
+const { isSpoofedSymbol } = require("./symbolSpoof");
 
 // Ethereum addresses are case-insensitive: EIP-55 mixed case is a checksum
 // over the address, not part of its identity.  Every address comparison in
@@ -245,18 +246,6 @@ async function fetchRecentTransactions(address, blockscoutUrl, count = 25) {
     return result;
 }
 
-// Check if a token transfer is spoofing a known symbol.
-// Returns true if the symbol matches a known token but the contract
-// address doesn't match the legitimate one.
-function isSpoofedSymbol(tx) {
-    if (!tx.contractAddress) return false;
-    const symbol = (tx.symbol || "").toUpperCase();
-    if (!KNOWN_SYMBOLS.has(symbol)) return false;
-    const legit = KNOWN_SYMBOLS.get(symbol);
-    if (legit === null) return true; // "ETH" as ERC-20 is always fake
-    return normalizeAddress(tx.contractAddress) !== normalizeAddress(legit);
-}
-
 // Pure filter function. Takes raw transactions and filter settings,
 // returns { transactions, newFraudContracts }.
 function filterTransactions(txs, filters = {}) {
@@ -283,7 +272,7 @@ function filterTransactions(txs, filters = {}) {
         const contract = normalizeAddress(tx.contractAddress);
 
         // Filter spoofed known symbols and record the fraud contract
-        if (hideSpoofed && isSpoofedSymbol(tx)) {
+        if (hideSpoofed && isSpoofedSymbol(tx.symbol, tx.contractAddress)) {
             if (contract && !fraudSet.has(contract)) {
                 fraudSet.add(contract);
                 newFraud.push(contract);

src/shared/walletDelete.js

@@ -1,5 +1,22 @@
-// Wallet deletion state transition, kept out of the view so the selection
+// Wallet and address deletion state transitions, kept out of the views so the
-// and broadcast rules are testable without a DOM.
+// selection and broadcast rules are testable without a DOM.
+
+// Two records of the same address can be stored in different cases, so
+// address equality is never a literal string comparison.
+function sameAddress(a, b) {
+    if (a === null || a === undefined || b === null || b === undefined) {
+        return false;
+    }
+    return String(a).toLowerCase() === String(b).toLowerCase();
+}
+
+// Forget every site permission held against the given addresses.
+function dropSitePermissions(state, addresses) {
+    for (const addr of addresses) {
+        delete state.allowedSites[addr];
+        delete state.deniedSites[addr];
+    }
+}
 
 // Remove wallet `walletIdx` from `state` and repair the derived state.
 //
@@ -18,19 +35,13 @@ function removeWalletFromState(state, walletIdx) {
     const wallet = state.wallets[walletIdx];
     const addresses = (wallet.addresses || []).map((a) => a.address);
     const previousActive = state.activeAddress;
-    const activeWasDeleted =
+    const activeWasDeleted = addresses.some((a) =>
-        previousActive !== null &&
+        sameAddress(a, previousActive),
-        previousActive !== undefined &&
+    );
-        addresses.some(
-            (a) => a.toLowerCase() === String(previousActive).toLowerCase(),
-        );
 
     state.wallets.splice(walletIdx, 1);
 
-    for (const addr of addresses) {
+    dropSitePermissions(state, addresses);
-        delete state.allowedSites[addr];
-        delete state.deniedSites[addr];
-    }
 
     state.hasWallet = state.wallets.length > 0;
 
@@ -58,6 +69,77 @@ function removeWalletFromState(state, walletIdx) {
     return { activeAddressChanged: state.activeAddress !== previousActive };
 }
 
+// Whether a wallet may be offered a per-address remove control, and the same
+// gate the removal itself is held behind.
+//
+// Only a wallet that derives its addresses from an extended key can hold more
+// than one, so only those get the control — a key wallet has exactly one
+// address and no "+" button either. The last address of any wallet is never
+// removable: a wallet with no addresses is what delete-wallet is for.
+function canRemoveAddress(wallet) {
+    if (!wallet) return false;
+    if (wallet.type !== "hd" && wallet.type !== "xprv") return false;
+    return (wallet.addresses || []).length > 1;
+}
+
+// Remove address `addrIdx` of wallet `walletIdx` and repair the derived state.
+//
+// Nothing is destroyed here. The address stays derivable from the wallet's own
+// key material and any funds at it are untouched; this only stops the wallet
+// tracking it. `nextIndex` is deliberately left alone — it is a derivation
+// high-water mark, so "+" derives a fresh index rather than handing back the
+// address just removed, and the gap it leaves is within what
+// `scanForAddresses()` re-discovers on a later import.
+//
+// The rules mirror removeWalletFromState() one level down:
+//   - The call is refused unless canRemoveAddress() allows it, so the last
+//     address of a wallet always survives.
+//   - Site permissions are dropped for the removed address.
+//   - `selectedAddress` follows the splice, but only within the wallet that
+//     lost the address: it is decremented when an earlier address was
+//     removed, and falls back to that wallet's first address when the
+//     selection itself was removed. `selectedWallet` never moves, because the
+//     wallet list does not.
+//   - `activeAddress` moves only when it was the removed address, and then to
+//     the wallet's first remaining address.
+//
+// Returns whether the address was removed and whether `activeAddress`
+// changed, so the caller can broadcast it.
+function removeAddressFromState(state, walletIdx, addrIdx) {
+    const wallet = state.wallets[walletIdx];
+    const refused = { removed: false, activeAddressChanged: false };
+    if (!canRemoveAddress(wallet)) return refused;
+    if (!wallet.addresses[addrIdx]) return refused;
+
+    const address = wallet.addresses[addrIdx].address;
+    const previousActive = state.activeAddress;
+    const activeWasRemoved = sameAddress(address, previousActive);
+
+    wallet.addresses.splice(addrIdx, 1);
+
+    dropSitePermissions(state, [address]);
+
+    if (state.selectedWallet === walletIdx) {
+        if (state.selectedAddress === addrIdx) {
+            state.selectedAddress = 0;
+        } else if (
+            typeof state.selectedAddress === "number" &&
+            state.selectedAddress > addrIdx
+        ) {
+            state.selectedAddress -= 1;
+        }
+    }
+
+    if (activeWasRemoved) {
+        state.activeAddress = wallet.addresses[0].address;
+    }
+
+    return {
+        removed: true,
+        activeAddressChanged: state.activeAddress !== previousActive,
+    };
+}
+
 // Tell the background the active address changed, so it re-emits
 // accountsChanged to connected sites. Same call shape as the address
 // switch in the home view.
@@ -67,4 +149,9 @@ function broadcastActiveChanged() {
     runtime.sendMessage({ type: "AUTISTMASK_ACTIVE_CHANGED" });
 }
 
-module.exports = { removeWalletFromState, broadcastActiveChanged };
+module.exports = {
+    canRemoveAddress,
+    removeAddressFromState,
+    removeWalletFromState,
+    broadcastActiveChanged,
+};

tests/approvalVerify.test.js

@@ -1,8 +1,28 @@
-const { Network, Transaction, Wallet } = require("ethers");
+const {
+    Network,
+    Transaction,
+    Wallet,
+    decodeRlp,
+    encodeRlp,
+} = require("ethers");
 const {
     verifySignedTx,
     verifySignature,
+    assertNoForbiddenFields,
+    assertNothingUnchecked,
+    assertCanonicalBytes,
     sameAddress,
+    failureIsRetryable,
+    describeTxFailure,
+    describeSigningFailure,
+    ALLOWED_TX_TYPES,
+    SERIALIZED_FIELDS,
+    FORBIDDEN_FIELDS,
+    TX_STAGE_SIGN,
+    TX_STAGE_VERIFY,
+    TX_STAGE_BROADCAST,
+    MAX_GAS_LIMIT,
+    MAX_FEE_PER_GAS,
 } = require("../src/shared/approvalVerify");
 const { getSignerForAddress } = require("../src/shared/wallet");
 
@@ -18,6 +38,10 @@ const other = new Wallet(OTHER_KEY);
 const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
 const OTHER_RECIPIENT = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
 
+// The chain id of the selected network, as networks.js carries it.
+const SELECTED = "0x1";
+const SEPOLIA = "0xaa36a7";
+
 // Approved parameters as a dApp would supply them over eth_sendTransaction.
 const TX_PARAMS = {
     from: signer.address,
@@ -27,25 +51,38 @@ const TX_PARAMS = {
     gas: "0x5208",
 };
 
+// The values populateTransaction() fills in when the dApp fixed none of them.
+const POPULATED = {
+    chainId: 1,
+    nonce: 7,
+    gasLimit: 100000n,
+    maxFeePerGas: 2000000000n,
+    maxPriorityFeePerGas: 1000000000n,
+    type: 2,
+};
+
 // Build a signable transaction from approved params. The popup does the same
 // thing through populateTransaction(); here the fields are fixed so the test
-// needs no provider.
+// needs no provider. `overrides` stands in for what a tampered or misbuilt
-function txFor(params) {
+// popup would put on the wire.
+function txFor(params, overrides) {
     return {
-        chainId: 1,
+        ...POPULATED,
-        nonce: 7,
-        gasLimit: 100000n,
-        maxFeePerGas: 2000000000n,
-        maxPriorityFeePerGas: 1000000000n,
-        type: 2,
         to: params.to,
         value: params.value === undefined ? 0n : BigInt(params.value),
         data: params.data || "0x",
+        ...(overrides || {}),
     };
 }
 
-async function signedFor(params, withWallet) {
+async function signedFor(params, withWallet, overrides) {
-    return (withWallet || signer).signTransaction(txFor(params));
+    return (withWallet || signer).signTransaction(txFor(params, overrides));
+}
+
+// Sign the approved transaction with one field changed from what was
+// populated, which is the shape of every tamper case below.
+async function signedWith(overrides) {
+    return signedFor(TX_PARAMS, signer, overrides);
 }
 
 describe("sameAddress", () => {
@@ -71,7 +108,7 @@ describe("sameAddress", () => {
 describe("verifySignedTx", () => {
     test("accepts the approved transaction signed by the approved address", async () => {
         const raw = await signedFor(TX_PARAMS);
-        const parsed = verifySignedTx(raw, TX_PARAMS, signer.address);
+        const parsed = verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED);
         expect(parsed.from).toBe(signer.address);
         expect(parsed.hash).toBe(Transaction.from(raw).hash);
     });
@@ -79,14 +116,16 @@ describe("verifySignedTx", () => {
     test("accepts a contract creation with no recipient", async () => {
         const params = { to: undefined, value: "0x0", data: "0x600160005500" };
         const raw = await signedFor(params);
-        expect(() => verifySignedTx(raw, params, signer.address)).not.toThrow();
+        expect(() =>
+            verifySignedTx(raw, params, signer.address, SELECTED),
+        ).not.toThrow();
     });
 
     test("accepts an absent value as zero", async () => {
         const approved = { to: RECIPIENT, data: "0x" };
         const raw = await signedFor(approved);
         expect(() =>
-            verifySignedTx(raw, approved, signer.address),
+            verifySignedTx(raw, approved, signer.address, SELECTED),
         ).not.toThrow();
     });
 
@@ -94,7 +133,7 @@ describe("verifySignedTx", () => {
         const approved = { to: RECIPIENT, value: "0x0", data: "0xDEADBEEF" };
         const raw = await signedFor(approved);
         expect(() =>
-            verifySignedTx(raw, approved, signer.address),
+            verifySignedTx(raw, approved, signer.address, SELECTED),
         ).not.toThrow();
     });
 
@@ -103,9 +142,9 @@ describe("verifySignedTx", () => {
             ...TX_PARAMS,
             to: OTHER_RECIPIENT,
         });
-        expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow(
+        expect(() =>
-            /approved recipient/,
+            verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
-        );
+        ).toThrow(/approved recipient/);
     });
 
     test("rejects an inflated value", async () => {
@@ -113,48 +152,48 @@ describe("verifySignedTx", () => {
             ...TX_PARAMS,
             value: "0x4563918244f40000",
         });
-        expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow(
+        expect(() =>
-            /approved value/,
+            verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
-        );
+        ).toThrow(/approved value/);
     });
 
     test("rejects substituted call data", async () => {
         const raw = await signedFor({ ...TX_PARAMS, data: "0xc0ffee" });
-        expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow(
+        expect(() =>
-            /approved call data/,
+            verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
-        );
+        ).toThrow(/approved call data/);
     });
 
     test("rejects a transaction signed by a different address", async () => {
         const raw = await signedFor(TX_PARAMS, other);
-        expect(() => verifySignedTx(raw, TX_PARAMS, signer.address)).toThrow(
+        expect(() =>
-            /different address/,
+            verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
-        );
+        ).toThrow(/different address/);
     });
 
     test("rejects an unsigned transaction", () => {
         const unsigned = Transaction.from(txFor(TX_PARAMS)).unsignedSerialized;
         expect(() =>
-            verifySignedTx(unsigned, TX_PARAMS, signer.address),
+            verifySignedTx(unsigned, TX_PARAMS, signer.address, SELECTED),
         ).toThrow(/no valid signature/);
     });
 
     test("rejects a missing or malformed payload", () => {
         expect(() =>
-            verifySignedTx(undefined, TX_PARAMS, signer.address),
+            verifySignedTx(undefined, TX_PARAMS, signer.address, SELECTED),
         ).toThrow(/missing or malformed/);
-        expect(() => verifySignedTx("nope", TX_PARAMS, signer.address)).toThrow(
-            /missing or malformed/,
-        );
         expect(() =>
-            verifySignedTx("0xc0ffee", TX_PARAMS, signer.address),
+            verifySignedTx("nope", TX_PARAMS, signer.address, SELECTED),
+        ).toThrow(/missing or malformed/);
+        expect(() =>
+            verifySignedTx("0xc0ffee", TX_PARAMS, signer.address, SELECTED),
         ).toThrow(/could not be decoded/);
     });
 
     test("every rejection message is a full sentence", async () => {
         const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT });
         try {
-            verifySignedTx(raw, TX_PARAMS, signer.address);
+            verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED);
             throw new Error("expected a rejection");
         } catch (e) {
             expect(e.message).toMatch(/^[A-Z].*\.$/);
@@ -162,6 +201,606 @@ describe("verifySignedTx", () => {
     });
 });
 
+// One case per consequential field: the field alone differs from what was
+// approved, and that alone must refuse the signature.
+describe("verifySignedTx field comparison", () => {
+    test("rejects a chain id that is not the selected network", async () => {
+        const raw = await signedWith({ chainId: 11155111 });
+        expect(() =>
+            verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
+        ).toThrow(/different network than the one that is selected/);
+    });
+
+    test("rejects a chain id that is not the approved one", async () => {
+        // Selected network and signed chain id agree; the dApp asked for a
+        // different chain, so the artifact is not what was approved.
+        const approved = { ...TX_PARAMS, chainId: SEPOLIA };
+        const raw = await signedWith({});
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).toThrow(/different network than the one that was approved/);
+    });
+
+    test("refuses when the selected network is unknown", async () => {
+        const raw = await signedWith({});
+        expect(() =>
+            verifySignedTx(raw, TX_PARAMS, signer.address, undefined),
+        ).toThrow(/selected network is unknown/);
+    });
+
+    test("rejects a substituted nonce", async () => {
+        const approved = { ...TX_PARAMS, nonce: 7 };
+        const raw = await signedWith({ nonce: 8 });
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).toThrow(/approved nonce/);
+    });
+
+    test("rejects a substituted gas limit", async () => {
+        const approved = { ...TX_PARAMS, gasLimit: "0x186a0" };
+        const raw = await signedWith({ gasLimit: 250000n });
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).toThrow(/approved gas limit/);
+    });
+
+    test("rejects a substituted maximum fee per gas", async () => {
+        const approved = { ...TX_PARAMS, maxFeePerGas: "0x77359400" };
+        const raw = await signedWith({ maxFeePerGas: 900000000000n });
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).toThrow(/approved maximum fee per gas/);
+    });
+
+    test("rejects a substituted maximum priority fee per gas", async () => {
+        const approved = { ...TX_PARAMS, maxPriorityFeePerGas: "0x3b9aca00" };
+        const raw = await signedWith({ maxPriorityFeePerGas: 1500000000n });
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).toThrow(/approved maximum priority fee per gas/);
+    });
+
+    test("rejects a substituted legacy gas price", async () => {
+        const approved = { ...TX_PARAMS, gasPrice: "0x77359400" };
+        const legacy = {
+            type: 0,
+            gasPrice: 9000000000n,
+            maxFeePerGas: null,
+            maxPriorityFeePerGas: null,
+        };
+        const raw = await signedWith(legacy);
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).toThrow(/approved gas price/);
+    });
+
+    test("rejects an approved legacy fee signed as an EIP-1559 fee", async () => {
+        const approved = { ...TX_PARAMS, gasPrice: "0x77359400" };
+        const raw = await signedWith({});
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).toThrow(/approved fee mechanism/);
+    });
+
+    test("rejects an approved EIP-1559 fee signed as a legacy fee", async () => {
+        const approved = { ...TX_PARAMS, maxFeePerGas: "0x77359400" };
+        const raw = await signedWith({
+            type: 0,
+            gasPrice: 2000000000n,
+            maxFeePerGas: null,
+            maxPriorityFeePerGas: null,
+        });
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).toThrow(/approved fee mechanism/);
+    });
+
+    test("rejects a gas limit above anything a supported network accepts", async () => {
+        const raw = await signedWith({ gasLimit: MAX_GAS_LIMIT + 1n });
+        expect(() =>
+            verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
+        ).toThrow(/gas limit no network this wallet supports/);
+    });
+
+    test("rejects an absurd fee per gas the approval never fixed", async () => {
+        const raw = await signedWith({
+            maxFeePerGas: MAX_FEE_PER_GAS + 1n,
+            maxPriorityFeePerGas: MAX_FEE_PER_GAS + 1n,
+        });
+        expect(() =>
+            verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
+        ).toThrow(/fee per gas far above any plausible value/);
+    });
+
+    test("every field mismatch is a refusal, not a warning", async () => {
+        const raw = await signedWith({ nonce: 8 });
+        try {
+            verifySignedTx(
+                raw,
+                { ...TX_PARAMS, nonce: 7 },
+                signer.address,
+                SELECTED,
+            );
+            throw new Error("expected a rejection");
+        } catch (e) {
+            expect(e.approvalMismatch).toBe(true);
+            expect(e.message).toMatch(/^[A-Z].*\.$/);
+        }
+    });
+});
+
+// The transaction type decides which fields exist, so an artifact of a type
+// this wallet does not sign carries consequences the approval cannot describe
+// and none of the field comparisons can see. The approval used here is the
+// ordinary dApp shape with no fee fields — the common case, since
+// populateTransaction() fills them — which is exactly the case the
+// fee-mechanism check cannot catch by accident.
+describe("verifySignedTx transaction type", () => {
+    const BARE_APPROVAL = {
+        from: signer.address,
+        to: RECIPIENT,
+        value: "0x2386f26fc10000",
+        data: "0x",
+    };
+
+    // An EIP-7702 artifact that pays the approved amount to the approved
+    // recipient and, in the same transaction, installs the attacker's code at
+    // the signer's own account for good. Every field the approval screen shows
+    // matches; only the type and the authorization list do not.
+    test("refuses a type 4 artifact that delegates the signer's own account", async () => {
+        const authorization = await signer.authorize({
+            address: OTHER_RECIPIENT,
+            chainId: 1,
+            nonce: 8,
+        });
+        const raw = await signedFor(BARE_APPROVAL, signer, {
+            type: 4,
+            authorizationList: [authorization],
+        });
+        const parsed = Transaction.from(raw);
+        expect(parsed.type).toBe(4);
+        expect(parsed.authorizationList[0].address).toBe(OTHER_RECIPIENT);
+        expect(() =>
+            verifySignedTx(raw, BARE_APPROVAL, signer.address, SELECTED),
+        ).toThrow(/type this wallet does not sign/);
+    });
+
+    test("refuses a type 3 blob artifact", async () => {
+        const raw = await signedFor(BARE_APPROVAL, signer, {
+            type: 3,
+            maxFeePerBlobGas: 1000000000n,
+            blobVersionedHashes: ["0x01" + "ab".repeat(31)],
+        });
+        expect(Transaction.from(raw).type).toBe(3);
+        expect(() =>
+            verifySignedTx(raw, BARE_APPROVAL, signer.address, SELECTED),
+        ).toThrow(/type this wallet does not sign/);
+    });
+
+    test("refuses every type outside the allowlist, not just the known ones", async () => {
+        for (const type of [3, 4]) {
+            expect(ALLOWED_TX_TYPES).not.toContain(type);
+        }
+        expect(ALLOWED_TX_TYPES).toEqual([0, 1, 2]);
+    });
+
+    test("a type refusal is a refusal, not a warning", async () => {
+        const authorization = await signer.authorize({
+            address: OTHER_RECIPIENT,
+            chainId: 1,
+            nonce: 8,
+        });
+        const raw = await signedFor(BARE_APPROVAL, signer, {
+            type: 4,
+            authorizationList: [authorization],
+        });
+        try {
+            verifySignedTx(raw, BARE_APPROVAL, signer.address, SELECTED);
+            throw new Error("expected a rejection");
+        } catch (e) {
+            expect(e.approvalMismatch).toBe(true);
+            expect(e.message).toMatch(/^[A-Z].*\.$/);
+        }
+    });
+
+    test("accepts a legacy type 0 transaction", async () => {
+        const approved = { ...BARE_APPROVAL, gasPrice: "0x77359400" };
+        const raw = await signedFor(approved, signer, {
+            type: 0,
+            gasPrice: 2000000000n,
+            maxFeePerGas: null,
+            maxPriorityFeePerGas: null,
+        });
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).not.toThrow();
+    });
+
+    test("accepts a type 1 transaction whose access list is the approved one", async () => {
+        const accessList = [{ address: OTHER_RECIPIENT, storageKeys: [] }];
+        const approved = {
+            ...BARE_APPROVAL,
+            gasPrice: "0x77359400",
+            accessList,
+        };
+        const raw = await signedFor(approved, signer, {
+            type: 1,
+            gasPrice: 2000000000n,
+            maxFeePerGas: null,
+            maxPriorityFeePerGas: null,
+            accessList,
+        });
+        expect(Transaction.from(raw).type).toBe(1);
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).not.toThrow();
+    });
+
+    test("refuses an access list the approval never carried", async () => {
+        const raw = await signedFor(BARE_APPROVAL, signer, {
+            accessList: [{ address: OTHER_RECIPIENT, storageKeys: [] }],
+        });
+        expect(() =>
+            verifySignedTx(raw, BARE_APPROVAL, signer.address, SELECTED),
+        ).toThrow(/approved access list/);
+    });
+
+    test("treats an absent access list and an empty one as the same thing", async () => {
+        const approved = { ...BARE_APPROVAL, accessList: [] };
+        const raw = await signedFor(BARE_APPROVAL, signer, {});
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).not.toThrow();
+    });
+});
+
+// The allowlist is only exhaustive while it accounts for every field an
+// artifact can carry. These tests are what makes that claim checkable rather
+// than asserted.
+describe("verifySignedTx exhaustiveness", () => {
+    // Every accessor ethers exposes on a parsed transaction, and where this
+    // module deals with it. If an ethers upgrade adds a transaction field,
+    // this fails and forces a decision about it instead of letting it default
+    // to unchecked.
+    test("every field ethers can parse is accounted for", () => {
+        const derived = [
+            // Recovered from the signature or computed from the payload, not
+            // independent content: covered by the signer check and by the
+            // fields below.
+            "from",
+            "fromPublicKey",
+            "hash",
+            "serialized",
+            "signature",
+            "type",
+            "typeName",
+            "unsignedHash",
+            "unsignedSerialized",
+            // Blob sidecar machinery, meaningful only alongside `blobs`,
+            // which is refused outright.
+            "kzg",
+            "blobWrapperVersion",
+        ];
+        const accounted = new Set([
+            ...derived,
+            ...FORBIDDEN_FIELDS.map((f) => f.key),
+            ...Object.values(SERIALIZED_FIELDS).flat(),
+        ]);
+        const exposed = Object.getOwnPropertyNames(Transaction.prototype)
+            .filter((name) => {
+                const d = Object.getOwnPropertyDescriptor(
+                    Transaction.prototype,
+                    name,
+                );
+                return d && typeof d.get === "function";
+            })
+            .sort();
+        expect(exposed.filter((name) => !accounted.has(name))).toEqual([]);
+    });
+
+    // The two layers behind the type allowlist. Nothing reachable through
+    // verifySignedTx can trip either of them while the allowlist holds — that
+    // is what they are for — so they are exercised directly rather than taken
+    // on trust.
+    test("a forbidden field is refused even on an allowed type", async () => {
+        const authorization = await signer.authorize({
+            address: OTHER_RECIPIENT,
+            chainId: 1,
+            nonce: 8,
+        });
+        const carriers = {
+            authorizationList: [authorization],
+            blobVersionedHashes: ["0x01" + "ab".repeat(31)],
+            blobs: ["0x00"],
+            maxFeePerBlobGas: 1n,
+        };
+        for (const key of Object.keys(carriers)) {
+            expect(FORBIDDEN_FIELDS.map((f) => f.key)).toContain(key);
+            let thrown;
+            try {
+                assertNoForbiddenFields({ type: 2, [key]: carriers[key] });
+                throw new Error("expected a rejection");
+            } catch (e) {
+                thrown = e;
+            }
+            expect(thrown.approvalMismatch).toBe(true);
+            expect(thrown.message).toMatch(/^[A-Z].*\.$/);
+        }
+        expect(() => assertNoForbiddenFields({ type: 2 })).not.toThrow();
+    });
+
+    // Stands in for a future ethers that parses a field this module does not
+    // know about onto an allowed type: every field the module checks is
+    // identical, and the bytes are not.
+    test("an artifact carrying more than the checked fields is refused", async () => {
+        const parsed = Transaction.from(await signedWith({}));
+        const smuggled = { type: parsed.type };
+        for (const key of SERIALIZED_FIELDS[parsed.type]) {
+            smuggled[key] = parsed[key];
+        }
+        smuggled.unsignedSerialized = parsed.unsignedSerialized + "ff";
+        expect(() => assertNothingUnchecked(smuggled)).toThrow(
+            /beyond the fields that were checked/,
+        );
+        expect(() => assertNothingUnchecked(parsed)).not.toThrow();
+    });
+
+    // The closing check rebuilds the artifact from the fields the module
+    // compared and compares the bytes, so an artifact carrying anything else
+    // is refused without the module having to name it. Assert the rebuild is
+    // faithful for every accepted shape, since a rebuild that dropped a
+    // legitimate field would refuse honest transactions.
+    test("an accepted artifact of each allowed type rebuilds byte for byte", async () => {
+        const shapes = [
+            {
+                approved: { ...TX_PARAMS, gasPrice: "0x77359400" },
+                overrides: {
+                    type: 0,
+                    gasPrice: 2000000000n,
+                    maxFeePerGas: null,
+                    maxPriorityFeePerGas: null,
+                },
+            },
+            {
+                approved: {
+                    ...TX_PARAMS,
+                    gasPrice: "0x77359400",
+                    accessList: [
+                        {
+                            address: RECIPIENT,
+                            storageKeys: ["0x" + "11".repeat(32)],
+                        },
+                    ],
+                },
+                overrides: {
+                    type: 1,
+                    gasPrice: 2000000000n,
+                    maxFeePerGas: null,
+                    maxPriorityFeePerGas: null,
+                    accessList: [
+                        {
+                            address: RECIPIENT,
+                            storageKeys: ["0x" + "11".repeat(32)],
+                        },
+                    ],
+                },
+            },
+            { approved: TX_PARAMS, overrides: {} },
+        ];
+        for (const shape of shapes) {
+            const raw = await signedFor(
+                shape.approved,
+                signer,
+                shape.overrides,
+            );
+            const parsed = verifySignedTx(
+                raw,
+                shape.approved,
+                signer.address,
+                SELECTED,
+            );
+            const fields = { type: parsed.type };
+            for (const key of SERIALIZED_FIELDS[parsed.type]) {
+                fields[key] = parsed[key];
+            }
+            expect(Transaction.from(fields).unsignedSerialized).toBe(
+                parsed.unsignedSerialized,
+            );
+        }
+    });
+});
+
+// Every comparison above runs against the decode, but the string that is
+// handed to broadcastTransaction() is the artifact. An encoding the decoder
+// normalizes away therefore checks as one transaction and broadcasts as
+// different bytes, so the artifact must be the canonical encoding of itself.
+describe("verifySignedTx canonical encoding", () => {
+    // Re-encode a signed type-2 artifact with a leading zero byte on the RLP
+    // value field. It decodes to exactly the approved transaction — same
+    // value, same signer, same everything the field comparisons look at — and
+    // it is not the same string.
+    async function nonCanonical() {
+        const raw = await signedWith({});
+        const items = decodeRlp("0x" + raw.slice(4));
+        // type 2 payload order: chainId, nonce, maxPriorityFeePerGas,
+        // maxFeePerGas, gasLimit, to, value, data, accessList, then the
+        // signature.
+        const padded = items.slice();
+        padded[6] = "0x00" + items[6].slice(2);
+        return "0x02" + encodeRlp(padded).slice(2);
+    }
+
+    test("the mutation decodes to the approved transaction and is not it", async () => {
+        const raw = await signedWith({});
+        const mutated = await nonCanonical();
+        const parsed = Transaction.from(mutated);
+        expect(mutated).not.toBe(raw);
+        expect(mutated.length).toBeGreaterThan(raw.length);
+        expect(parsed.value).toBe(BigInt(TX_PARAMS.value));
+        expect(parsed.from).toBe(signer.address);
+        expect(parsed.serialized).not.toBe(mutated);
+    });
+
+    test("refuses an artifact that is not its own canonical encoding", async () => {
+        const mutated = await nonCanonical();
+        expect(() =>
+            verifySignedTx(mutated, TX_PARAMS, signer.address, SELECTED),
+        ).toThrow(/not encoded canonically/);
+    });
+
+    test("assertCanonicalBytes accepts what ethers itself produced", async () => {
+        const raw = await signedWith({});
+        expect(() =>
+            assertCanonicalBytes(Transaction.from(raw), raw),
+        ).not.toThrow();
+    });
+
+    test("hex case is not part of the encoding", async () => {
+        const raw = await signedWith({});
+        const upper = "0x" + raw.slice(2).toUpperCase();
+        expect(() =>
+            verifySignedTx(upper, TX_PARAMS, signer.address, SELECTED),
+        ).not.toThrow();
+    });
+});
+
+// The approval and the artifact spell the same values differently. None of
+// these differences is tampering, so none may refuse the signature.
+describe("verifySignedTx normalization", () => {
+    test("accepts a decimal chain id against a hex selected network", async () => {
+        const raw = await signedWith({});
+        expect(() =>
+            verifySignedTx(raw, TX_PARAMS, signer.address, 1),
+        ).not.toThrow();
+        expect(() =>
+            verifySignedTx(raw, TX_PARAMS, signer.address, "1"),
+        ).not.toThrow();
+    });
+
+    test("accepts an approved chain id written in hex", async () => {
+        const raw = await signedWith({});
+        const approved = { ...TX_PARAMS, chainId: "0x1" };
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).not.toThrow();
+    });
+
+    test("accepts a hex nonce against a numeric one", async () => {
+        const raw = await signedWith({ nonce: 7 });
+        expect(() =>
+            verifySignedTx(
+                raw,
+                { ...TX_PARAMS, nonce: "0x7" },
+                signer.address,
+                SELECTED,
+            ),
+        ).not.toThrow();
+    });
+
+    test("accepts a decimal gas limit against a hex one", async () => {
+        const raw = await signedWith({ gasLimit: 100000n });
+        expect(() =>
+            verifySignedTx(
+                raw,
+                { ...TX_PARAMS, gasLimit: "100000" },
+                signer.address,
+                SELECTED,
+            ),
+        ).not.toThrow();
+    });
+
+    test("accepts fee fields spelled as hex, decimal, number and bigint", async () => {
+        const raw = await signedWith({});
+        for (const maxFee of [
+            "0x77359400",
+            "2000000000",
+            2000000000,
+            2000000000n,
+        ]) {
+            expect(() =>
+                verifySignedTx(
+                    raw,
+                    { ...TX_PARAMS, maxFeePerGas: maxFee },
+                    signer.address,
+                    SELECTED,
+                ),
+            ).not.toThrow();
+        }
+    });
+
+    test("accepts an approval that fixes no nonce, gas or fee at all", async () => {
+        const raw = await signedWith({});
+        expect(() =>
+            verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED),
+        ).not.toThrow();
+    });
+
+    test("accepts an approval whose recipient case differs", async () => {
+        const raw = await signedWith({});
+        const approved = { ...TX_PARAMS, to: RECIPIENT.toLowerCase() };
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).not.toThrow();
+    });
+
+    test("accepts absent call data against 0x", async () => {
+        const approved = { to: RECIPIENT, value: "0x0" };
+        const raw = await signedFor({ ...approved, data: "0x" });
+        expect(() =>
+            verifySignedTx(raw, approved, signer.address, SELECTED),
+        ).not.toThrow();
+    });
+
+    test("refuses an approved quantity that is not a number", async () => {
+        const raw = await signedWith({});
+        expect(() =>
+            verifySignedTx(
+                raw,
+                { ...TX_PARAMS, maxFeePerGas: "cheap" },
+                signer.address,
+                SELECTED,
+            ),
+        ).toThrow(/is not a number/);
+    });
+
+    // The value is page-controlled. A refusal is correct; a raw BigInt
+    // conversion error is not, because it is not a mismatch, so it would be
+    // reported retryable and leave the approval unspent behind a live button
+    // that can never succeed.
+    test("refuses an approved value that is not a number, as a mismatch", async () => {
+        const raw = await signedWith({});
+        for (const value of ["cheap", 1.5, "1e18", {}]) {
+            let thrown;
+            try {
+                verifySignedTx(
+                    raw,
+                    { ...TX_PARAMS, value },
+                    signer.address,
+                    SELECTED,
+                );
+                throw new Error("expected a rejection");
+            } catch (e) {
+                thrown = e;
+            }
+            expect(thrown.approvalMismatch).toBe(true);
+            expect(thrown.message).toMatch(/approved value is not a number/);
+            expect(failureIsRetryable(thrown)).toBe(false);
+        }
+    });
+
+    test("refuses an approved access list that is not an access list", async () => {
+        const raw = await signedWith({});
+        expect(() =>
+            verifySignedTx(
+                raw,
+                { ...TX_PARAMS, accessList: ["nope"] },
+                signer.address,
+                SELECTED,
+            ),
+        ).toThrow(/not a valid access list/);
+    });
+});
+
 const TYPED_DATA = JSON.stringify({
     domain: {
         name: "AutistMask Test",
@@ -281,6 +920,147 @@ describe("verifySignature", () => {
     });
 });
 
+// What happens after a signing attempt fails: the background keeps the
+// approval for anything the user can correct, and the popup only offers the
+// button again when it did.
+describe("signing failure and retry", () => {
+    test("a failure that is not a mismatch leaves the approval retryable", () => {
+        expect(failureIsRetryable(new Error("The node is unreachable."))).toBe(
+            true,
+        );
+        expect(failureIsRetryable(undefined)).toBe(true);
+    });
+
+    test("a mismatch spends the approval", async () => {
+        const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT });
+        try {
+            verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED);
+            throw new Error("expected a rejection");
+        } catch (e) {
+            expect(failureIsRetryable(e)).toBe(false);
+        }
+    });
+
+    test("a retryable failure keeps the button usable and says only what failed", () => {
+        const outcome = describeSigningFailure(
+            { error: "The node rejected the transaction.", retryable: true },
+            "The transaction could not be sent.",
+        );
+        expect(outcome.retryable).toBe(true);
+        expect(outcome.message).toBe("The node rejected the transaction.");
+    });
+
+    test("a refusal tells the user to start again from the site", () => {
+        const outcome = describeSigningFailure(
+            {
+                error: "The signed transaction does not go to the approved recipient.",
+                retryable: false,
+            },
+            "The transaction could not be sent.",
+        );
+        expect(outcome.retryable).toBe(false);
+        expect(outcome.message).toMatch(/start it again from the site\.$/);
+    });
+
+    test("a refusal for an attempt already running does not say to start again", () => {
+        const outcome = describeSigningFailure(
+            {
+                error: "This request is already being signed.",
+                retryable: false,
+                stage: "inflight",
+            },
+            "The message could not be signed.",
+        );
+        expect(outcome.retryable).toBe(false);
+        expect(outcome.message).not.toMatch(/start it again from the site/);
+        expect(outcome.message).toMatch(/first attempt is still running/);
+    });
+
+    test("a response the background never sent is treated as a spent approval", () => {
+        const outcome = describeSigningFailure(
+            undefined,
+            "The transaction could not be sent.",
+        );
+        expect(outcome.retryable).toBe(false);
+        expect(outcome.message).toMatch(/^The transaction could not be sent\./);
+    });
+
+    test("every failure message is a full sentence", () => {
+        const outcome = describeSigningFailure(
+            { error: "The node is on fire", retryable: true },
+            "The transaction could not be sent.",
+        );
+        expect(outcome.message).toMatch(/^[A-Z].*\.$/);
+    });
+
+    test("a popup that could not sign leaves the approval standing", () => {
+        const outcome = describeTxFailure(
+            TX_STAGE_SIGN,
+            "That password is incorrect. Please try again.",
+        );
+        expect(outcome.retryable).toBe(true);
+        expect(outcome.spendApproval).toBe(false);
+        expect(outcome.error).toMatch(/password is incorrect/);
+    });
+
+    test("a mismatch found at verification spends the approval", async () => {
+        const raw = await signedFor({ ...TX_PARAMS, to: OTHER_RECIPIENT });
+        let outcome;
+        try {
+            verifySignedTx(raw, TX_PARAMS, signer.address, SELECTED);
+        } catch (e) {
+            outcome = describeTxFailure(TX_STAGE_VERIFY, e);
+        }
+        expect(outcome.retryable).toBe(false);
+        expect(outcome.spendApproval).toBe(true);
+    });
+
+    test("a failure before the check ran is still retryable", () => {
+        const outcome = describeTxFailure(
+            TX_STAGE_VERIFY,
+            new Error("The wallet state could not be read."),
+        );
+        expect(outcome.retryable).toBe(true);
+        expect(outcome.spendApproval).toBe(false);
+    });
+
+    // A broadcast that throws after the node took the transaction is routine:
+    // a timeout, a dropped response, a node answering "already known". The
+    // popup's retry does not re-broadcast the same bytes — it re-populates and
+    // re-signs at a freshly fetched nonce — so a retryable broadcast failure
+    // would put the approved transfer on the chain twice.
+    test("a failed broadcast is terminal, whatever the node said", () => {
+        for (const message of [
+            "already known",
+            "timeout of 30000ms exceeded",
+            "could not coalesce error",
+            "replacement transaction underpriced",
+        ]) {
+            const outcome = describeTxFailure(
+                TX_STAGE_BROADCAST,
+                new Error(message),
+            );
+            expect(outcome.retryable).toBe(false);
+            expect(outcome.spendApproval).toBe(true);
+            expect(outcome.error).toBe(message);
+        }
+    });
+
+    test("a failed broadcast does not tell the user to send it again", () => {
+        const outcome = describeSigningFailure(
+            {
+                error: "The node did not answer.",
+                retryable: false,
+                stage: TX_STAGE_BROADCAST,
+            },
+            "The transaction could not be sent.",
+        );
+        expect(outcome.retryable).toBe(false);
+        expect(outcome.message).toMatch(/may still have reached the network/);
+        expect(outcome.message).not.toMatch(/start it again from the site/);
+    });
+});
+
 // End-to-end over the messaging boundary, without a browser: run the exact
 // sequence the approval popup runs, then hand the artifact to the exact check
 // the background runs before it broadcasts or resolves. Only what the popup
@@ -314,7 +1094,12 @@ describe("popup signing sequence to background verification", () => {
 
     test("a populated, signed transaction is accepted and broadcastable", async () => {
         const rawSignedTx = await popupSignsTx(TX_PARAMS);
-        const parsed = verifySignedTx(rawSignedTx, TX_PARAMS, signer.address);
+        const parsed = verifySignedTx(
+            rawSignedTx,
+            TX_PARAMS,
+            signer.address,
+            SELECTED,
+        );
         expect(parsed.nonce).toBe(7);
         expect(parsed.chainId).toBe(1n);
         expect(parsed.gasLimit).toBe(21000n);
@@ -349,7 +1134,14 @@ describe("popup signing sequence to background verification", () => {
             to: OTHER_RECIPIENT,
         });
         expect(() =>
-            verifySignedTx(rawSignedTx, TX_PARAMS, signer.address),
+            verifySignedTx(rawSignedTx, TX_PARAMS, signer.address, SELECTED),
         ).toThrow(/approved recipient/);
     });
+
+    test("the background rejects a transaction populated on another network", async () => {
+        const rawSignedTx = await popupSignsTx(TX_PARAMS);
+        expect(() =>
+            verifySignedTx(rawSignedTx, TX_PARAMS, signer.address, SEPOLIA),
+        ).toThrow(/different network than the one that is selected/);
+    });
 });

tests/backgroundApproval.test.js

@@ -0,0 +1,679 @@
+// The background's approval message wiring, driven end to end: a dApp
+// eth_sendTransaction raises a pending approval, and the popup answers it with
+// AUTISTMASK_TX_RESPONSE / AUTISTMASK_SIGN_RESPONSE.
+//
+// What this exists for is the duplicate response. The handler verifies and
+// broadcasts asynchronously, and the approval deliberately survives a
+// retryable failure so the user can try again with the transaction they
+// already saw — which means the entry being present is not by itself proof
+// that no attempt is running. A second response carrying the same id (a
+// reloaded approval window re-rendering a live Approve button, a popup that
+// emits the message twice) must not start a second verify and broadcast: with
+// the ordinary dApp approval shape the page fixes no nonce, so two artifacts
+// signed at different nonces both verify, and the approved transfer would go
+// out twice.
+
+const { Wallet } = require("ethers");
+
+const SIGNER_KEY =
+    "0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d";
+const signer = new Wallet(SIGNER_KEY);
+const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
+
+const ORIGIN = "https://dapp.example";
+const HOSTNAME = "dapp.example";
+const EXT_URL = "chrome-extension://autistmask/";
+
+// What the dApp asks for: no nonce, no gas, no fees. This is the shape that
+// makes a duplicate broadcast possible at all.
+const TX_PARAMS = {
+    from: signer.address,
+    to: RECIPIENT,
+    value: "0x2386f26fc10000",
+    data: "0x",
+};
+
+// The fields the popup's populateTransaction() would fill in. The nonce is a
+// parameter because the duplicate case turns on the two artifacts differing
+// in exactly the field nothing constrains.
+function populated(nonce) {
+    return {
+        type: 2,
+        chainId: 1,
+        nonce,
+        gasLimit: 100000n,
+        maxFeePerGas: 2000000000n,
+        maxPriorityFeePerGas: 1000000000n,
+        to: TX_PARAMS.to,
+        value: BigInt(TX_PARAMS.value),
+        data: TX_PARAMS.data,
+    };
+}
+
+function signedAtNonce(nonce) {
+    return signer.signTransaction(populated(nonce));
+}
+
+// A promise whose settlement the test controls, so a broadcast can be held in
+// flight while the second response arrives.
+function deferred() {
+    let resolve;
+    let reject;
+    const promise = new Promise((res, rej) => {
+        resolve = res;
+        reject = rej;
+    });
+    return { promise, resolve, reject };
+}
+
+// Load the background worker against stubbed browser and network APIs and
+// return the handles the tests drive it through. Everything that would touch
+// the network or the browser's own schedulers is mocked; the approval
+// verification is the real module, because that is what the handler under
+// test is wired to.
+function loadBackground(options) {
+    const opts = options || {};
+    jest.resetModules();
+
+    const broadcastTransaction = jest.fn();
+    const loadState = jest.fn(opts.loadState || (async () => {}));
+
+    jest.doMock("../src/shared/state", () => ({
+        state: { rpcUrl: "https://rpc.invalid", wallets: [] },
+        loadState,
+        saveState: jest.fn(async () => {}),
+        currentNetwork: () => ({ chainId: "0x1" }),
+    }));
+    jest.doMock("../src/shared/balances", () => ({
+        getProvider: () => ({ broadcastTransaction }),
+        refreshBalances: jest.fn(async () => {}),
+    }));
+    jest.doMock("../src/shared/phishingDomains", () => ({
+        isPhishingDomain: () => false,
+        refreshPhishingListOnSchedule: jest.fn(async () => {}),
+        initPhishingList: jest.fn(async () => {}),
+    }));
+    jest.doMock("../src/shared/alarms", () => ({
+        BALANCE_REFRESH_ALARM: "balance",
+        PHISHING_REFRESH_ALARM: "phishing",
+        BALANCE_REFRESH_PERIOD_MINUTES: 1,
+        ensureRecurringAlarms: jest.fn(async () => {}),
+        registerAlarmHandlers: jest.fn(),
+    }));
+
+    const persisted = {
+        wallets: [
+            { name: "Wallet 1", type: "hd", addresses: [signer.address] },
+        ],
+        rpcUrl: "https://rpc.invalid",
+        activeAddress: signer.address,
+        allowedSites: { [signer.address]: [HOSTNAME] },
+        deniedSites: {},
+    };
+
+    let messageListener = null;
+    let windowRemovedListener = null;
+    const created = [];
+    const removed = [];
+
+    global.chrome = {
+        storage: {
+            local: {
+                get: jest.fn(async () => ({ autistmask: persisted })),
+                set: jest.fn(async () => {}),
+            },
+        },
+        runtime: {
+            getURL: (path) => EXT_URL + path,
+            onMessage: {
+                addListener: (fn) => {
+                    messageListener = fn;
+                },
+            },
+            onConnect: { addListener: () => {} },
+            lastError: null,
+        },
+        windows: {
+            getLastFocused: (cb) => cb(null),
+            create: (options2, cb) => {
+                created.push(options2);
+                cb({ id: created.length });
+            },
+            remove: (id, cb) => {
+                removed.push(id);
+                if (cb) cb();
+            },
+            // Captured, not swallowed: closing the approval window is the
+            // event that used to retire an approval out from under a live
+            // broadcast, and a no-op stub here hides exactly that.
+            onRemoved: {
+                addListener: (fn) => {
+                    windowRemovedListener = fn;
+                },
+            },
+        },
+        tabs: {
+            query: (q, cb) => cb([]),
+            sendMessage: () => {},
+        },
+        action: { setPopup: () => {} },
+    };
+
+    require("../src/background/index");
+
+    // Send a message the way the browser would, and hand back whatever the
+    // handler passed to sendResponse.
+    function send(msg, sender) {
+        const sendResponse = jest.fn();
+        const kept = messageListener(msg, sender || {}, sendResponse);
+        return { sendResponse, kept };
+    }
+
+    // Raise a pending transaction approval the way a dApp does, and dig the
+    // approval id back out of the popup URL the background opened.
+    function requestTx() {
+        let rpcResult = null;
+        const sendResponse = jest.fn((r) => {
+            rpcResult = r;
+        });
+        messageListener(
+            {
+                type: "AUTISTMASK_RPC",
+                method: "eth_sendTransaction",
+                params: [TX_PARAMS],
+            },
+            { origin: ORIGIN },
+            sendResponse,
+        );
+        return {
+            id: () => new URL(created[0].url).searchParams.get("approval"),
+            result: () => rpcResult,
+        };
+    }
+
+    // The user closes the approval popup. `created` is index-aligned with the
+    // ids the window stub hands back, so window 1 is the first popup opened.
+    function closeWindow(windowId) {
+        windowRemovedListener(windowId);
+    }
+
+    return {
+        send,
+        requestTx,
+        closeWindow,
+        broadcastTransaction,
+        loadState,
+        created,
+        removed,
+        fromPopup: { url: EXT_URL + "src/popup/index.html" },
+    };
+}
+
+// Let the handler's promise chain run to the next suspension point.
+async function settle() {
+    for (let i = 0; i < 10; i++) await Promise.resolve();
+}
+
+afterEach(() => {
+    delete global.chrome;
+    jest.resetModules();
+});
+
+describe("one approval, one broadcast", () => {
+    test("a second AUTISTMASK_TX_RESPONSE for the same id does not broadcast again", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+        expect(id).toBeTruthy();
+
+        const inFlight = deferred();
+        bg.broadcastTransaction.mockReturnValue(inFlight.promise);
+
+        // The popup answers. Verification passes and the broadcast is held
+        // open, which is the whole window the second message arrives in.
+        const first = bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(7),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
+
+        // A reloaded approval window signs the same approval again. Nothing
+        // in the approval fixes a nonce, so this artifact verifies just as
+        // well as the first one.
+        const second = bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(8),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+
+        expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
+        expect(second.sendResponse).toHaveBeenCalledWith(
+            expect.objectContaining({
+                error: expect.stringMatching(/already being sent/),
+                retryable: false,
+            }),
+        );
+
+        inFlight.resolve({ hash: "0xfeed" });
+        await settle();
+        expect(first.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
+        expect(pending.result()).toEqual({ result: "0xfeed" });
+        expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
+    });
+
+    test("the same artifact sent twice broadcasts once", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+
+        const inFlight = deferred();
+        bg.broadcastTransaction.mockReturnValue(inFlight.promise);
+        const raw = await signedAtNonce(7);
+        const msg = {
+            type: "AUTISTMASK_TX_RESPONSE",
+            id,
+            approved: true,
+            rawSignedTx: raw,
+        };
+
+        bg.send(msg, { url: bg.fromPopup.url });
+        bg.send(msg, { url: bg.fromPopup.url });
+        await settle();
+        inFlight.resolve({ hash: "0xfeed" });
+        await settle();
+
+        expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
+    });
+
+    test("a response arriving after the broadcast finished finds nothing to send", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+
+        bg.broadcastTransaction.mockResolvedValue({ hash: "0xfeed" });
+        bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(7),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+
+        const late = bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(8),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+
+        expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
+        expect(late.sendResponse).not.toHaveBeenCalled();
+    });
+
+    test("a second AUTISTMASK_SIGN_RESPONSE for the same id is refused", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+
+        // Hold the transaction approval in flight, then answer it a second
+        // time as if it were a sign approval: the sign handler must apply the
+        // same interlock rather than running its own verification.
+        const inFlight = deferred();
+        bg.broadcastTransaction.mockReturnValue(inFlight.promise);
+        bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(7),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+
+        const second = bg.send(
+            {
+                type: "AUTISTMASK_SIGN_RESPONSE",
+                id,
+                approved: true,
+                signature: "0x00",
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+
+        expect(second.sendResponse).toHaveBeenCalledWith(
+            expect.objectContaining({
+                error: expect.stringMatching(/already being signed/),
+                retryable: false,
+            }),
+        );
+        inFlight.resolve({ hash: "0xfeed" });
+        await settle();
+        expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
+    });
+});
+
+// The interlock must not cost the retry the approval exists to allow.
+describe("the interlock releases a failed attempt", () => {
+    test("a retryable failure before the broadcast leaves the approval usable", async () => {
+        let failNext = true;
+        const bg = loadBackground({
+            loadState: async () => {
+                if (failNext) {
+                    failNext = false;
+                    throw new Error("storage unavailable");
+                }
+            },
+        });
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+
+        const first = bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(7),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(bg.broadcastTransaction).not.toHaveBeenCalled();
+        expect(first.sendResponse).toHaveBeenCalledWith(
+            expect.objectContaining({ retryable: true }),
+        );
+
+        bg.broadcastTransaction.mockResolvedValue({ hash: "0xfeed" });
+        const retry = bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(7),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+
+        expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
+        expect(retry.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
+        expect(pending.result()).toEqual({ result: "0xfeed" });
+    });
+
+    test("a mismatched artifact spends the approval outright", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+
+        // Signed for a different recipient than the one that was approved.
+        const wrong = await signer.signTransaction({
+            ...populated(7),
+            to: "0xdAC17F958D2ee523a2206206994597C13D831ec7",
+        });
+        const first = bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: wrong,
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(first.sendResponse).toHaveBeenCalledWith(
+            expect.objectContaining({ retryable: false, stage: "verify" }),
+        );
+
+        const retry = bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(7),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+
+        expect(bg.broadcastTransaction).not.toHaveBeenCalled();
+        expect(retry.sendResponse).not.toHaveBeenCalled();
+    });
+});
+
+// The claim is what makes one approval one broadcast, so it has to hold
+// against everything else that retires an approval, not just against a second
+// AUTISTMASK_TX_RESPONSE. Each of these paths used to resolve the waiting
+// promise 4001 while the attempt behind it ran to completion: the transaction
+// reached the chain and the page was told the user rejected it, which invites
+// the user to send it a second time at a fresh nonce.
+describe("a claimed approval outlives every other retirement path", () => {
+    // The approval popup stays open across the broadcast it is waiting on, so
+    // a user closing an apparently-hung window needs no adversary at all.
+    test("closing the approval window mid-broadcast still reports the result", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+
+        const inFlight = deferred();
+        bg.broadcastTransaction.mockReturnValue(inFlight.promise);
+        const first = bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(7),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
+
+        // The user closes the window while the broadcast is still open.
+        bg.closeWindow(1);
+        await settle();
+        expect(pending.result()).toBeNull();
+
+        inFlight.resolve({ hash: "0xfeed" });
+        await settle();
+
+        expect(pending.result()).toEqual({ result: "0xfeed" });
+        expect(first.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
+        expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
+    });
+
+    test("switching the active address mid-broadcast still reports the result", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+
+        const inFlight = deferred();
+        bg.broadcastTransaction.mockReturnValue(inFlight.promise);
+        bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(7),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
+
+        // The user switches account in the toolbar popup, which rejects and
+        // force-closes every pending approval.
+        bg.send(
+            { type: "AUTISTMASK_ACTIVE_CHANGED" },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(pending.result()).toBeNull();
+        // The window an in-flight attempt reports into is left standing too.
+        expect(bg.removed).toEqual([]);
+
+        inFlight.resolve({ hash: "0xfeed" });
+        await settle();
+
+        expect(pending.result()).toEqual({ result: "0xfeed" });
+        expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
+    });
+
+    test("a reject arriving mid-broadcast is refused, not honoured", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+
+        const inFlight = deferred();
+        bg.broadcastTransaction.mockReturnValue(inFlight.promise);
+        bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(7),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+
+        const reject = bg.send(
+            { type: "AUTISTMASK_TX_RESPONSE", id, approved: false },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(pending.result()).toBeNull();
+        expect(reject.sendResponse).toHaveBeenCalledWith(
+            expect.objectContaining({
+                retryable: false,
+                stage: "broadcast",
+            }),
+        );
+
+        inFlight.resolve({ hash: "0xfeed" });
+        await settle();
+
+        expect(pending.result()).toEqual({ result: "0xfeed" });
+        expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
+    });
+
+    // The refusals above must not cost the rejection its ordinary meaning.
+    test("with no attempt running, closing the window still rejects", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+
+        bg.closeWindow(1);
+        await settle();
+
+        expect(pending.result()).toEqual({
+            error: { code: 4001, message: "User rejected the request." },
+        });
+        expect(bg.broadcastTransaction).not.toHaveBeenCalled();
+    });
+
+    test("with no attempt running, an active-address switch still rejects and closes", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+
+        bg.send(
+            { type: "AUTISTMASK_ACTIVE_CHANGED" },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+
+        expect(pending.result()).toEqual({
+            error: { code: 4001, message: "User rejected the request." },
+        });
+        expect(bg.removed).toEqual([1]);
+    });
+
+    // A sign approval held by a running verification is the same shape, and
+    // the refusal must not tell the user to start again from the site while
+    // the first attempt may still hand back a signature.
+    test("a reject during a sign attempt is refused with the in-flight stage", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+
+        const inFlight = deferred();
+        bg.broadcastTransaction.mockReturnValue(inFlight.promise);
+        bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(7),
+            },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+
+        const reject = bg.send(
+            { type: "AUTISTMASK_SIGN_RESPONSE", id, approved: false },
+            { url: bg.fromPopup.url },
+        );
+        await settle();
+        expect(reject.sendResponse).toHaveBeenCalledWith(
+            expect.objectContaining({ retryable: false, stage: "inflight" }),
+        );
+
+        inFlight.resolve({ hash: "0xfeed" });
+        await settle();
+        expect(pending.result()).toEqual({ result: "0xfeed" });
+    });
+});
+
+describe("popup-only messages", () => {
+    test("a page sender cannot answer an approval", async () => {
+        const bg = loadBackground();
+        const pending = bg.requestTx();
+        await settle();
+        const id = pending.id();
+
+        const spoof = bg.send(
+            {
+                type: "AUTISTMASK_TX_RESPONSE",
+                id,
+                approved: true,
+                rawSignedTx: await signedAtNonce(7),
+            },
+            { url: ORIGIN + "/index.html" },
+        );
+        await settle();
+
+        expect(bg.broadcastTransaction).not.toHaveBeenCalled();
+        expect(spoof.sendResponse).toHaveBeenCalledWith({
+            error: "Unauthorized sender",
+        });
+    });
+});

tests/deleteAddress.test.js

@@ -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:");
+    });
+});

tests/e2e/firefox/Dockerfile

@@ -0,0 +1,51 @@
+# Firefox end-to-end image: stock Firefox plus geckodriver on a node base,
+# built by script/test-e2e-firefox. The repo is bind-mounted at /work; the
+# harness itself has no dependencies, so nothing is installed for it.
+#
+# All three external artifacts are pinned by digest. The Firefox version in
+# particular must not float: -remote-allow-system-access is mandatory on 153
+# and was not on 142, so the flag the harness passes is version-coupled.
+
+# node:22-bookworm-slim, 2026-08-12
+FROM node@sha256:d649c27dae7ba0137b3cef5dd75baa422c08dc3d9e3fc0c23dfb172dc3cc6436
+
+ENV DEBIAN_FRONTEND=noninteractive
+
+# Firefox's shared-library dependencies on a slim base, plus the two tools
+# needed to fetch and unpack the pinned tarballs.
+RUN apt-get update \
+    && apt-get install -y --no-install-recommends \
+        ca-certificates \
+        curl \
+        libasound2 \
+        libdbus-glib-1-2 \
+        libgtk-3-0 \
+        libx11-xcb1 \
+        libxt6 \
+        libxtst6 \
+        xz-utils \
+    && rm -rf /var/lib/apt/lists/*
+
+# Firefox 153.0.3, linux-x86_64, en-US
+ARG FIREFOX_URL=https://ftp.mozilla.org/pub/firefox/releases/153.0.3/linux-x86_64/en-US/firefox-153.0.3.tar.xz
+ARG FIREFOX_SHA256=22b312280900bfb174b685ece32c7b3c6d72e7f8e53d6d30f21ac41a8dc500a2
+RUN curl -fsSL -o /tmp/firefox.tar.xz "$FIREFOX_URL" \
+    && echo "$FIREFOX_SHA256  /tmp/firefox.tar.xz" | sha256sum -c - \
+    && tar -xJf /tmp/firefox.tar.xz -C /opt \
+    && rm /tmp/firefox.tar.xz \
+    && /opt/firefox/firefox --version
+
+# geckodriver v0.36.0, linux64
+ARG GECKODRIVER_URL=https://github.com/mozilla/geckodriver/releases/download/v0.36.0/geckodriver-v0.36.0-linux64.tar.gz
+ARG GECKODRIVER_SHA256=0bde38707eb0a686a20c6bd50f4adcc7d60d4f73c60eb83ee9e0db8f65823e04
+RUN curl -fsSL -o /tmp/geckodriver.tar.gz "$GECKODRIVER_URL" \
+    && echo "$GECKODRIVER_SHA256  /tmp/geckodriver.tar.gz" | sha256sum -c - \
+    && tar -xzf /tmp/geckodriver.tar.gz -C /usr/local/bin \
+    && rm /tmp/geckodriver.tar.gz \
+    && geckodriver --version
+
+ENV FIREFOX_BIN=/opt/firefox/firefox
+ENV GECKODRIVER=/usr/local/bin/geckodriver
+
+WORKDIR /work
+CMD ["node", "tests/e2e/firefox/run.js", "dist/firefox"]

tests/e2e/firefox/driver.js

@@ -0,0 +1,462 @@
+// A minimal WebDriver client for geckodriver, plus the privileged console
+// reader the error assertions are built on. No npm dependencies: global
+// fetch and child_process against geckodriver's HTTP API is less code than
+// a driver library and keeps the harness at zero packages.
+//
+// Run through script/test-e2e-firefox, which builds dist/firefox/ and the
+// pinned container around this. FIREFOX_BIN and GECKODRIVER locate the two
+// binaries; the image sets both.
+
+"use strict";
+
+const { spawn } = require("child_process");
+const net = require("net");
+
+const FIREFOX_BIN = process.env.FIREFOX_BIN || "firefox";
+const GECKODRIVER = process.env.GECKODRIVER || "geckodriver";
+
+// The extension id declared in manifest/firefox.json, and the uuid the
+// popup is served from. Firefox normally assigns that uuid randomly per
+// profile, which would make the popup URL undiscoverable without querying
+// privileged state; setting extensions.webextensions.uuids before launch
+// pins it instead. This only works because the manifest declares a fixed
+// browser_specific_settings.gecko.id — without one the mapping has no key.
+const EXTENSION_ID = "autistmask@sneak.berlin";
+const EXTENSION_UUID = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee";
+const EXTENSION_ORIGIN = "moz-extension://" + EXTENSION_UUID;
+
+// The W3C web element identifier. Getting the last character wrong yields
+// an element reference of "undefined" and a bewildering "element with the
+// reference undefined is not known" from geckodriver, so findElement()
+// below checks for the key rather than indexing blindly.
+const WEB_ELEMENT_KEY = "element-6066-11e4-a52e-4f735466cecf";
+
+const SCRIPT_TIMEOUT_MS = 120000;
+const DEFAULT_WAIT_MS = 20000;
+const POLL_INTERVAL_MS = 100;
+
+function sleep(ms) {
+    return new Promise((resolve) => setTimeout(resolve, ms));
+}
+
+// An ephemeral port picked by the kernel, then handed to geckodriver.
+// There is a race between closing this listener and geckodriver binding,
+// but this host runs many sessions at once and a fixed 4444 is a
+// guaranteed collision rather than a possible one.
+function freePort() {
+    return new Promise((resolve, reject) => {
+        const srv = net.createServer();
+        srv.on("error", reject);
+        srv.listen(0, "127.0.0.1", () => {
+            const { port } = srv.address();
+            srv.close(() => resolve(port));
+        });
+    });
+}
+
+class WebDriverError extends Error {
+    constructor(command, body) {
+        const v = (body && body.value) || {};
+        super(
+            command +
+                " failed: " +
+                (v.error || "unknown error") +
+                ": " +
+                (v.message || JSON.stringify(body)),
+        );
+        this.name = "WebDriverError";
+        this.error = v.error;
+    }
+}
+
+class Driver {
+    constructor(proc, base) {
+        this.proc = proc;
+        this.base = base;
+        this.sessionId = null;
+        this.context = "content";
+    }
+
+    async send(method, path, body) {
+        const url = this.base + path;
+        const res = await fetch(url, {
+            method,
+            headers: { "Content-Type": "application/json" },
+            body: body === undefined ? undefined : JSON.stringify(body),
+        });
+        const text = await res.text();
+        let parsed;
+        try {
+            parsed = JSON.parse(text);
+        } catch (_) {
+            throw new Error(
+                method + " " + path + ": non-JSON response: " + text,
+            );
+        }
+        if (!res.ok) throw new WebDriverError(method + " " + path, parsed);
+        return parsed.value;
+    }
+
+    session(method, path, body) {
+        return this.send(method, "/session/" + this.sessionId + path, body);
+    }
+
+    // ------------------------------------------------------------ setup
+
+    async newSession() {
+        const prefs = {
+            // See EXTENSION_UUID above. The pref is a string pref whose
+            // value is itself JSON.
+            "extensions.webextensions.uuids": JSON.stringify({
+                [EXTENSION_ID]: EXTENSION_UUID,
+            }),
+        };
+
+        const value = await this.send("POST", "/session", {
+            capabilities: {
+                alwaysMatch: {
+                    browserName: "firefox",
+                    "moz:firefoxOptions": {
+                        binary: FIREFOX_BIN,
+                        args: [
+                            "-headless",
+                            // Mandatory on Firefox 153: without it,
+                            // navigating to moz-extension:// and running
+                            // chrome-context script both fail with
+                            // "unsupported operation".
+                            //
+                            // It grants the driver FULL CHROME PRIVILEGES
+                            // over this browser. Acceptable only because
+                            // the browser is a throwaway in a CI
+                            // container; never point a session with this
+                            // flag at anything you care about.
+                            "-remote-allow-system-access",
+                        ],
+                        prefs,
+                    },
+                },
+            },
+        });
+        this.sessionId = value.sessionId;
+        await this.session("POST", "/timeouts", { script: SCRIPT_TIMEOUT_MS });
+        return value;
+    }
+
+    // Installs the unpacked MV2 build straight from a directory.
+    // temporary:true bypasses signature checks, so no XPI and no signing
+    // are involved, and the add-on dies with the profile.
+    async installAddon(dir) {
+        return this.session("POST", "/moz/addon/install", {
+            path: dir,
+            temporary: true,
+        });
+    }
+
+    // Classic navigation on purpose. BiDi's browsingContext.navigate
+    // refuses moz-extension:// URLs outright.
+    async navigate(url) {
+        await this.session("POST", "/url", { url });
+    }
+
+    async quit() {
+        if (this.sessionId) {
+            await this.session("DELETE", "").catch(() => {});
+            this.sessionId = null;
+        }
+        this.proc.kill("SIGTERM");
+    }
+
+    // ---------------------------------------------------------- scripts
+
+    async setContext(context) {
+        if (this.context === context) return;
+        await this.session("POST", "/moz/context", { context });
+        this.context = context;
+    }
+
+    async execute(script, args = []) {
+        await this.setContext("content");
+        return this.session("POST", "/execute/sync", { script, args });
+    }
+
+    // Runs in the privileged chrome scope, where Services and Ci exist.
+    async executeChrome(script, args = []) {
+        await this.setContext("chrome");
+        try {
+            return await this.session("POST", "/execute/sync", {
+                script,
+                args,
+            });
+        } finally {
+            await this.setContext("content");
+        }
+    }
+
+    // ------------------------------------------------------- page waits
+
+    // Polls a content-context expression until it returns truthy. Every
+    // wait in the suite goes through here so a timeout always says which
+    // condition it was waiting on rather than "timed out".
+    async waitFor(what, script, args = [], timeout = DEFAULT_WAIT_MS) {
+        const deadline = Date.now() + timeout;
+        let last = null;
+        for (;;) {
+            try {
+                const v = await this.execute(script, args);
+                if (v) return v;
+                last = null;
+            } catch (e) {
+                // A navigation or view swap in flight makes execute
+                // throw; that is a not-yet, not a failure, until the
+                // deadline says otherwise.
+                last = e.message;
+            }
+            if (Date.now() >= deadline) {
+                throw new Error(
+                    "timed out after " +
+                        timeout +
+                        "ms waiting for " +
+                        what +
+                        (last ? " (last error: " + last + ")" : ""),
+                );
+            }
+            await sleep(POLL_INTERVAL_MS);
+        }
+    }
+
+    // Shown means shown: in the popup a view is switched by toggling a
+    // "hidden" class, and an element that is present but collapsed is not
+    // the thing a test means by visible.
+    async waitVisible(selector, timeout = DEFAULT_WAIT_MS) {
+        return this.waitFor(
+            "selector " + selector + " to be visible",
+            `const el = document.querySelector(arguments[0]);
+             if (!el) return false;
+             const r = el.getBoundingClientRect();
+             return r.width > 0 && r.height > 0;`,
+            [selector],
+            timeout,
+        );
+    }
+
+    async isVisible(selector) {
+        return this.execute(
+            `const el = document.querySelector(arguments[0]);
+             if (!el) return false;
+             const r = el.getBoundingClientRect();
+             return r.width > 0 && r.height > 0;`,
+            [selector],
+        );
+    }
+
+    async count(selector) {
+        return this.execute(
+            "return document.querySelectorAll(arguments[0]).length;",
+            [selector],
+        );
+    }
+
+    async text(selector) {
+        return this.execute(
+            `const el = document.querySelector(arguments[0]);
+             return el ? el.textContent : null;`,
+            [selector],
+        );
+    }
+
+    async title() {
+        return this.session("GET", "/title");
+    }
+
+    // The id of the view element currently on top, which is what a
+    // failing step needs to report: "the screen did not change" is only
+    // useful if it says which screen it stayed on.
+    async currentView() {
+        return this.execute(
+            `const views = document.querySelectorAll('[id^="view-"]');
+             for (const v of views) {
+                 const r = v.getBoundingClientRect();
+                 if (r.width > 0 && r.height > 0) return v.id;
+             }
+             return null;`,
+        );
+    }
+
+    // ----------------------------------------------------- interactions
+
+    async findElement(selector) {
+        const value = await this.session("POST", "/element", {
+            using: "css selector",
+            value: selector,
+        });
+        const ref = value && value[WEB_ELEMENT_KEY];
+        if (typeof ref !== "string") {
+            throw new Error(
+                "no " +
+                    WEB_ELEMENT_KEY +
+                    " in the element response for " +
+                    selector +
+                    ": " +
+                    JSON.stringify(value),
+            );
+        }
+        return ref;
+    }
+
+    // Real WebDriver clicks and real key events rather than in-page
+    // .click() and value assignment: the popup's handlers are wired to
+    // events, and synthesising them from inside the page would test the
+    // harness's idea of the UI instead of the UI.
+    async click(selector) {
+        await this.waitVisible(selector);
+        const id = await this.findElement(selector);
+        await this.session("POST", "/element/" + id + "/click", {});
+    }
+
+    async fill(selector, value) {
+        await this.waitVisible(selector);
+        const id = await this.findElement(selector);
+        await this.session("POST", "/element/" + id + "/clear", {});
+        await this.session("POST", "/element/" + id + "/value", {
+            text: String(value),
+        });
+    }
+
+    async value(selector) {
+        return this.execute(
+            `const el = document.querySelector(arguments[0]);
+             return el ? el.value : null;`,
+            [selector],
+        );
+    }
+}
+
+// ------------------------------------------------------- error capture
+
+// Uncaught errors from extension code, read out of the privileged console
+// service.
+//
+// This is not the obvious mechanism, and the obvious one does not work:
+// WebDriver BiDi's log.entryAdded delivers NOTHING for extension pages.
+// Verified on Firefox 142 and 153 against a same-session control — a plain
+// http:// page yields uncaught errors with stack traces, the
+// moz-extension:// popup yields zero events, because the remote agent
+// excludes extension browsing contexts from BiDi observation. A harness
+// built on Playwright-BiDi or Puppeteer-BiDi therefore sees nothing and
+// reports success. Do not "simplify" this back to BiDi.
+//
+// nsIConsoleService is not per-page: it also carries errors from the
+// background page, which BiDi would not have covered even if it worked.
+// Background-page capture is verified by probe — a throw at the top of
+// src/background/index.js, which kills the background page outright, fails
+// the run. Content-script errors should arrive by the same route, but that
+// is UNVERIFIED here and must not be claimed: the container runs with
+// --network none, so there is no http:// page for a content script to be
+// injected into and this suite never exercises one.
+//
+// Warnings are excluded so the semantics match Playwright's pageerror:
+// uncaught errors only.
+//
+// The read and the clear are ONE chrome script on purpose. Splitting them
+// into two round trips leaves a blind window between them in which an
+// error is logged into a buffer that is about to be discarded, and is
+// destroyed unread rather than deferred to the next drain. That was not
+// theoretical: with a separate reset() call, a probe of 100 sequenced
+// throws at 20ms spacing lost one of them outright.
+const DRAIN_ERRORS_SCRIPT = `
+    const origin = arguments[0];
+    const out = [];
+    for (const raw of Services.console.getMessageArray() || []) {
+        let e;
+        try {
+            e = raw.QueryInterface(Ci.nsIScriptError);
+        } catch (_) {
+            continue;
+        }
+        if (e.flags & Ci.nsIScriptError.warningFlag) continue;
+        const src = e.sourceName || "";
+        if (!src.startsWith(origin)) continue;
+        out.push({
+            msg: e.errorMessage,
+            src: src,
+            line: e.lineNumber,
+            cat: e.category,
+        });
+    }
+    Services.console.reset();
+    return out;
+`;
+
+class ConsoleErrors {
+    constructor(driver, originPrefix) {
+        this.driver = driver;
+        this.originPrefix = originPrefix;
+    }
+
+    // Everything logged since the last take, read and cleared atomically
+    // in a single chrome round trip. Poll-based, so an error is attributed
+    // to the step that was running when it was drained, not to the moment
+    // inside that step at which it happened — see the limitation note in
+    // run.js. Nothing between two takes is lost, though: an error that
+    // arrives mid-drain either makes this batch or the next one.
+    async take() {
+        const found = await this.driver.executeChrome(DRAIN_ERRORS_SCRIPT, [
+            this.originPrefix,
+        ]);
+        return found || [];
+    }
+}
+
+// ------------------------------------------------------------- startup
+
+async function waitForDriverReady(base, timeoutMs) {
+    const deadline = Date.now() + timeoutMs;
+    for (;;) {
+        try {
+            const res = await fetch(base + "/status");
+            if (res.ok) {
+                const body = await res.json();
+                if (body && body.value && body.value.ready !== false) return;
+            }
+        } catch (_) {
+            // not listening yet
+        }
+        if (Date.now() >= deadline) {
+            throw new Error(
+                "geckodriver did not become ready within " + timeoutMs + "ms",
+            );
+        }
+        await sleep(POLL_INTERVAL_MS);
+    }
+}
+
+async function start() {
+    const port = await freePort();
+    const proc = spawn(
+        GECKODRIVER,
+        ["--port", String(port), "--host", "127.0.0.1"],
+        { stdio: ["ignore", "inherit", "inherit"] },
+    );
+    proc.on("error", (e) => {
+        console.error("geckodriver failed to spawn: " + e.message);
+    });
+
+    const base = "http://127.0.0.1:" + port;
+    try {
+        await waitForDriverReady(base, 30000);
+    } catch (e) {
+        proc.kill("SIGKILL");
+        throw e;
+    }
+    return new Driver(proc, base);
+}
+
+module.exports = {
+    ConsoleErrors,
+    Driver,
+    EXTENSION_ID,
+    EXTENSION_ORIGIN,
+    EXTENSION_UUID,
+    start,
+    sleep,
+};

tests/e2e/firefox/run.js

@@ -0,0 +1,284 @@
+// Firefox end-to-end suite: drives the real popup in a real Firefox with
+// the unpacked MV2 build installed as a temporary add-on, and fails the run
+// on any uncaught error coming from an extension source.
+//
+// Run via script/test-e2e-firefox, which builds dist/firefox/ and the pinned
+// container. The extension directory is the one argument.
+//
+//   node tests/e2e/firefox/run.js [dist/firefox]
+//
+// Deliberately not part of script/check, and deliberately not named
+// *.test.js: REPO_POLICIES.md caps make test at 20 seconds and a browser
+// suite does not fit.
+//
+// This shares no driver layer with the Chrome suite in tests/e2e/, and the
+// UI steps below are written twice on purpose. Chrome runs on Playwright,
+// which cannot see extension-page errors in Firefox at all (see the BiDi
+// note in driver.js), so the two backends have no common substrate to
+// abstract over. Three duplicated steps do not pay for a shim; revisit if
+// this suite grows to where they do.
+//
+// LIMITATION, and the difference from the Chrome suite worth knowing: error
+// capture here is POLL-BASED, not event-streamed. The console service is
+// drained at each step boundary, so an error is attributed to the step it
+// was drained after, never to a moment within that step. What is drained
+// covers the whole run from add-on install to the last drain below, which
+// measures out at ~1.5s after the last step returns — errors at +0.5s,
+// +1.0s and +1.5s are reported, +1.6s and later never are, because the
+// browser is torn down first. Nothing inside that window is dropped: the
+// drain reads and clears in one chrome round trip, so there is no gap for
+// an error to be destroyed unread in. The Chrome harness receives
+// pageerror events as they happen and can say more. Do not read a green
+// Firefox run as the same claim.
+
+"use strict";
+
+const fs = require("fs");
+const path = require("path");
+
+const { ConsoleErrors, EXTENSION_ORIGIN, start, sleep } = require("./driver");
+
+const REPO_ROOT = path.resolve(__dirname, "..", "..", "..");
+const POPUP_URL = EXTENSION_ORIGIN + "/src/popup/index.html";
+const PASSWORD = "e2e-harness-password";
+
+// Firefox installs the add-on and starts its background page asynchronously
+// after the install call returns. Nothing observable marks the end of that,
+// so the popup's own first render is the signal we wait on instead.
+const STEP_TIMEOUT_MS = 120000;
+
+const steps = [];
+
+function step(name, fn) {
+    steps.push({ name, fn });
+}
+
+function assert(cond, message) {
+    if (!cond) throw new Error(message);
+}
+
+function withTimeout(promise, name) {
+    let timer;
+    const timeout = new Promise((_, reject) => {
+        timer = setTimeout(
+            () =>
+                reject(
+                    new Error(
+                        name + " timed out after " + STEP_TIMEOUT_MS + "ms",
+                    ),
+                ),
+            STEP_TIMEOUT_MS,
+        );
+    });
+    return Promise.race([promise, timeout]).finally(() => clearTimeout(timer));
+}
+
+// ------------------------------------------------------------- steps
+
+step("popup loads and reaches the welcome view", async (env) => {
+    const d = env.driver;
+    await d.navigate(POPUP_URL);
+    await d.waitVisible("#view-welcome", STEP_TIMEOUT_MS);
+    const title = await d.title();
+    assert(title === "AutistMask", "unexpected popup title: " + title);
+});
+
+step("wallet creation through the UI reaches the main view", async (env) => {
+    const d = env.driver;
+    await d.click("#btn-welcome-add");
+    await d.waitVisible("#view-add-wallet");
+    await d.click("#btn-generate-phrase");
+    await d.waitFor(
+        "a generated recovery phrase of at least 12 words",
+        `const el = document.getElementById("wallet-mnemonic");
+         return !!el && el.value.trim().split(/\\s+/).length >= 12;`,
+    );
+    env.phrase = (await d.value("#wallet-mnemonic")).trim();
+
+    await d.fill("#add-wallet-password", PASSWORD);
+    await d.fill("#add-wallet-password-confirm", PASSWORD);
+    await d.click("#btn-add-wallet-confirm");
+    // Argon2id under libsodium, for real, so this is the slow one.
+    await d.waitVisible("#view-main", STEP_TIMEOUT_MS);
+
+    assert(
+        env.phrase.split(/\s+/).length >= 12,
+        "wallet creation did not yield a recovery phrase",
+    );
+    const addrs = await d.count("#wallet-list .btn-addr-info");
+    assert(addrs > 0, "no addresses rendered in the wallet list");
+});
+
+step("add token screen opens from address detail", async (env) => {
+    const d = env.driver;
+    if (!(await d.isVisible("#view-address"))) {
+        await d.waitVisible("#view-main");
+        await d.click("#wallet-list .btn-addr-info");
+    }
+    await d.waitVisible("#view-address");
+
+    await d.click("#btn-add-token");
+    // Reported with the view it actually stayed on: a screen that does
+    // not change is the symptom a missing import produces, and naming
+    // the screen is what makes that diagnosable.
+    try {
+        await d.waitVisible("#view-add-token");
+    } catch (e) {
+        throw new Error(
+            e.message + "; current view is " + (await d.currentView()),
+        );
+    }
+
+    const picks = await d.count("#common-token-list .common-token");
+    assert(picks > 0, "no common-token quick-pick buttons rendered");
+});
+
+// ------------------------------------------------------------- runner
+
+function formatError(e) {
+    return (
+        e.msg + " (" + e.src + ":" + e.line + (e.cat ? ", " + e.cat : "") + ")"
+    );
+}
+
+async function main() {
+    // A suite that runs nothing must never report success.
+    if (steps.length === 0) {
+        console.log("1..0");
+        console.log("# FAILED: the Firefox e2e suite registered no steps");
+        process.exitCode = 1;
+        return;
+    }
+
+    const extDir = path.resolve(REPO_ROOT, process.argv[2] || "dist/firefox");
+    if (!fs.existsSync(path.join(extDir, "manifest.json"))) {
+        console.error(
+            "e2e-firefox: no unpacked build at " +
+                extDir +
+                " — run make build first",
+        );
+        process.exitCode = 1;
+        return;
+    }
+
+    let driver;
+    try {
+        driver = await start();
+        await driver.newSession();
+        await driver.installAddon(extDir);
+    } catch (e) {
+        // A browser we cannot start is a failure of the suite, not an
+        // absent suite. Never skip and report success.
+        console.error("e2e-firefox: cannot run the suite: " + e.message);
+        if (driver) await driver.quit().catch(() => {});
+        process.exitCode = 1;
+        return;
+    }
+
+    const errors = new ConsoleErrors(driver, EXTENSION_ORIGIN);
+    const env = { driver, phrase: null };
+
+    console.log("# extension origin: " + EXTENSION_ORIGIN);
+    console.log("1.." + steps.length);
+
+    let failed = 0;
+    let n = 0;
+    try {
+        // Drain, never reset: anything the add-on logged while installing
+        // and starting its background page has no earlier step to belong
+        // to, so it is folded into step 1 below. Services.console.reset()
+        // here would DELETE it instead, and a background page that throws
+        // at the top of the file — a dead background page — would then
+        // produce a fully green run.
+        let installErrors = [];
+        let installFailure = null;
+        try {
+            installErrors = await errors.take();
+        } catch (e) {
+            installFailure =
+                "could not read the console after install: " + e.message;
+        }
+
+        for (const s of steps) {
+            n += 1;
+            let failure = null;
+            try {
+                await withTimeout(s.fn(env), s.name);
+            } catch (e) {
+                failure = e.message;
+            }
+
+            // Let anything the step provoked reach the console service
+            // before draining it. Without this a failure logged on the
+            // way out of the step lands in the next step's drain, which
+            // still fails the run but blames the wrong step.
+            await sleep(500);
+
+            let found = [];
+            try {
+                found = await errors.take();
+            } catch (e) {
+                failure = failure || "could not read the console: " + e.message;
+            }
+
+            if (n === 1) {
+                found = installErrors.concat(found);
+                installErrors = [];
+                failure = failure || installFailure;
+                installFailure = null;
+            }
+
+            // Any uncaught error from an extension source fails the step
+            // that provoked it, whether or not its assertions passed.
+            if (!failure && found.length > 0) {
+                failure =
+                    n === 1
+                        ? "uncaught extension errors during add-on install, " +
+                          "background startup or this step"
+                        : "uncaught extension errors during this step";
+            }
+
+            if (failure) {
+                failed += 1;
+                console.log("not ok " + n + " - " + s.name);
+                console.log("  " + failure);
+                for (const e of found) console.log("  " + formatError(e));
+            } else {
+                console.log("ok " + n + " - " + s.name);
+            }
+        }
+
+        // The tail: errors logged after the last step returned cannot be
+        // blamed on any one step, but they are still reported and they
+        // still fail the run.
+        await sleep(1000);
+        const trailing = await errors.take();
+        console.log(
+            "# " +
+                (steps.length - failed) +
+                "/" +
+                steps.length +
+                " steps passed",
+        );
+        if (trailing.length > 0) {
+            console.log(
+                "# " +
+                    trailing.length +
+                    " extension error(s) recorded after the last step, not " +
+                    "attributable to any single step:",
+            );
+            for (const e of trailing) console.log("#   " + formatError(e));
+        }
+        if (failed > 0 || trailing.length > 0) {
+            console.log("# FAILED");
+            process.exitCode = 1;
+        }
+    } finally {
+        await driver.quit().catch(() => {});
+    }
+}
+
+main().catch((e) => {
+    console.error("e2e-firefox: " + (e && e.stack ? e.stack : e));
+    process.exitCode = 1;
+});

tests/e2e/run.js

@@ -398,6 +398,101 @@ test("reopening the popup never lands on the phrase screen (#161)", async (env)
     assertWiped(st, env.phrase, "after reopening the popup");
 });
 
+// -------------------------------------------- address removal (#162)
+
+// Number of address rows across every wallet in the list, counted in the DOM
+// whether or not Home is the screen on top.
+function addressRowCount(page) {
+    return page.locator("#wallet-list .btn-addr-info").count();
+}
+
+function waitForAddressRows(page, n) {
+    return page.waitForFunction(
+        (want) =>
+            document.querySelectorAll("#wallet-list .btn-addr-info").length ===
+            want,
+        n,
+        { timeout: 60000 },
+    );
+}
+
+// The suite arrives here with two wallets, an HD one and a key one, holding
+// one address each.
+test("only a wallet that can spare an address offers to remove one (#162)", async (env) => {
+    await visible(env.page, "#view-main");
+    const rows = await addressRowCount(env.page);
+    assert(rows === 2, "expected two address rows, got " + rows);
+    const offered = await env.page
+        .locator("#wallet-list .btn-remove-address")
+        .count();
+    assert(
+        offered === 0,
+        "a wallet holding its last address offered to remove it",
+    );
+
+    await env.page.click("#wallet-list .btn-add-address");
+    await waitForAddressRows(env.page, 3);
+
+    // Only the HD wallet's two rows; the key wallet still holds one address.
+    const nowOffered = await env.page
+        .locator("#wallet-list .btn-remove-address")
+        .count();
+    assert(
+        nowOffered === 2,
+        "expected the HD wallet's two rows to offer removal, got " + nowOffered,
+    );
+});
+
+// The gate itself: the control opens a confirmation, and leaving that
+// confirmation by "Back" removes nothing.
+test("leaving the removal confirmation removes nothing (#162)", async (env) => {
+    await env.page.locator("#wallet-list .btn-remove-address").nth(1).click();
+    await visible(env.page, "#view-delete-address-confirm");
+
+    const label = await env.page.locator("#delete-address-label").innerText();
+    assert(
+        label === "Address 2",
+        "the confirmation names the wrong address: " + JSON.stringify(label),
+    );
+
+    // The route back is written by the view, not by index.html, so an empty
+    // paragraph here means the user is confirming with no idea what it
+    // takes to undo. This wallet is an HD one, so it is told about its
+    // recovery phrase.
+    const recovery = await env.page
+        .locator("#delete-address-recovery")
+        .innerText();
+    assert(
+        recovery.includes("delete the whole wallet in Settings") &&
+            recovery.includes("recovery phrase"),
+        "the confirmation does not state the route back: " +
+            JSON.stringify(recovery),
+    );
+
+    // "Back" re-renders Home, so a count taken after it is a real
+    // measurement of the wallet rather than a stale screen.
+    await env.page.click("#btn-delete-address-back");
+    await visible(env.page, "#view-main");
+    const rows = await addressRowCount(env.page);
+    assert(rows === 3, "the address was removed without a confirmation");
+});
+
+test("confirming removes the address and returns Home (#162)", async (env) => {
+    await env.page.locator("#wallet-list .btn-remove-address").nth(1).click();
+    await visible(env.page, "#view-delete-address-confirm");
+    await env.page.click("#btn-delete-address-confirm");
+    await visible(env.page, "#view-main");
+
+    await waitForAddressRows(env.page, 2);
+    const offered = await env.page
+        .locator("#wallet-list .btn-remove-address")
+        .count();
+    assert(
+        offered === 0,
+        "the HD wallet still offers to remove its last address",
+    );
+});
+
 // ---------------------------------------------------------------- runner
 
 async function main() {

tests/symbolSpoof.test.js

@@ -0,0 +1,296 @@
+// Tests for the known-symbol spoof rule (src/shared/symbolSpoof.js) and for
+// its application on all three surfaces that show tokens: the transaction
+// history, the Send token selector, and the balance list.
+//
+// Issue #235: the three surfaces disagreed about what a `null` entry in
+// KNOWN_SYMBOLS means.  The history and the selector read it as "no contract
+// may bear this symbol" and filtered a fake `ETH` ERC-20; the balance list
+// read it as "no comparison is possible" and listed the fake token next to
+// the user's real ETH, which is where a user forms their belief about what
+// they own.  The rule now lives in one module, so a fourth surface cannot
+// reintroduce a fourth reading, and these tests assert the same attack on
+// each surface.
+//
+// Nothing here touches the network: global.fetch is a throwing stub and the
+// only fetch path in the modules under test (debugFetch, from
+// src/shared/log) is mocked at the module boundary.
+
+// The RPC provider is replaced so that refreshBalances can be driven end to
+// end: the native balance it reports must survive a balance list in which
+// every ERC-20 row is a fake ETH.  Everything else in ethers is the real
+// module, including the formatters the assertions depend on.
+jest.mock("ethers", () => {
+    const actual = jest.requireActual("ethers");
+    class StubProvider {
+        async getBalance() {
+            return 1234500000000000000n;
+        }
+        async lookupAddress() {
+            return null;
+        }
+    }
+    return {
+        ...actual,
+        JsonRpcProvider: StubProvider,
+        Network: { from: () => ({}) },
+    };
+});
+
+jest.mock("../src/shared/log", () => ({
+    log: {
+        debugf: () => {},
+        infof: () => {},
+        warnf: () => {},
+        errorf: () => {},
+    },
+    debugFetch: jest.fn(),
+    setRuntimeDebug: () => {},
+    isDebug: () => false,
+}));
+
+global.fetch = jest.fn(() => {
+    throw new Error("tests must not perform network requests");
+});
+global.chrome = {
+    storage: { local: { get: async () => ({}), set: async () => {} } },
+};
+
+const { isSpoofedSymbol } = require("../src/shared/symbolSpoof");
+const { KNOWN_SYMBOLS } = require("../src/shared/tokenList");
+const { filterTransactions } = require("../src/shared/transactions");
+const {
+    fetchTokenBalances,
+    refreshBalances,
+} = require("../src/shared/balances");
+const { renderSendTokenSelect } = require("../src/popup/views/send");
+const { state } = require("../src/shared/state");
+const { debugFetch } = require("../src/shared/log");
+
+// The fake "Ethereum" token with symbol "ETH" from the attack documented in
+// README.md, given a holder count high enough to clear every other filter so
+// that only the known-symbol rule can catch it.
+const FAKE_ETH_CONTRACT = "0xd05339f9ea5ab9d9f03b9d57f671d2abd1f55c82";
+const HOLDER = "0x66133e8ea0f5d1d612d2502a968757d1048c214a";
+const USDC_CONTRACT = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
+const WETH_CONTRACT = "0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2";
+const BLOCKSCOUT = "https://eth.blockscout.com/api/v2";
+
+describe("the shared rule", () => {
+    test('"ETH" is still the null-mapped symbol these tests assume', () => {
+        expect(KNOWN_SYMBOLS.get("ETH")).toBeNull();
+    });
+
+    test("a contract bearing a null-mapped symbol is a spoof", () => {
+        expect(isSpoofedSymbol("ETH", FAKE_ETH_CONTRACT)).toBe(true);
+    });
+
+    test("even a genuine contract may not bear a null-mapped symbol", () => {
+        expect(isSpoofedSymbol("ETH", WETH_CONTRACT)).toBe(true);
+    });
+
+    test("the native asset carries no contract and is never a spoof", () => {
+        expect(isSpoofedSymbol("ETH", null)).toBe(false);
+        expect(isSpoofedSymbol("ETH", undefined)).toBe(false);
+        expect(isSpoofedSymbol("ETH", "")).toBe(false);
+    });
+
+    // The native exemption is "has no contract address", not "the symbol is
+    // ETH".  A second null-mapped symbol added to the table later inherits
+    // both halves of the rule without any call site being revisited.
+    test("a newly null-mapped symbol behaves the same way", () => {
+        const added = !KNOWN_SYMBOLS.has("XTZTEST");
+        KNOWN_SYMBOLS.set("XTZTEST", null);
+        try {
+            expect(isSpoofedSymbol("XTZTEST", FAKE_ETH_CONTRACT)).toBe(true);
+            expect(isSpoofedSymbol("XTZTEST", null)).toBe(false);
+        } finally {
+            if (added) KNOWN_SYMBOLS.delete("XTZTEST");
+        }
+    });
+
+    test("a known symbol from its own contract is not a spoof", () => {
+        expect(isSpoofedSymbol("USDC", USDC_CONTRACT)).toBe(false);
+        expect(isSpoofedSymbol("usdc", USDC_CONTRACT.toUpperCase())).toBe(
+            false,
+        );
+    });
+
+    test("a known symbol from another contract is a spoof", () => {
+        expect(isSpoofedSymbol("USDC", FAKE_ETH_CONTRACT)).toBe(true);
+    });
+
+    test("a symbol that is not in the table is not judged here", () => {
+        expect(isSpoofedSymbol("SPAMTKN", FAKE_ETH_CONTRACT)).toBe(false);
+    });
+});
+
+describe("surface 1: the transaction history", () => {
+    function fakeEthTransfer() {
+        return {
+            hash: "0x" + "1".repeat(64),
+            symbol: "ETH",
+            contractAddress: FAKE_ETH_CONTRACT,
+            holders: 900000,
+            valueGwei: null,
+            isContractCall: false,
+        };
+    }
+
+    test("a fake ETH token transfer is filtered", () => {
+        const result = filterTransactions([fakeEthTransfer()], {
+            hideSpoofedSymbols: true,
+            hideFraudContracts: true,
+            hideLowHolderTokens: true,
+            hideDustTransactions: true,
+            dustThresholdGwei: 100000,
+        });
+        expect(result.transactions).toEqual([]);
+    });
+
+    test("a real native ETH transfer survives", () => {
+        const native = {
+            hash: "0x" + "2".repeat(64),
+            symbol: "ETH",
+            contractAddress: null,
+            holders: null,
+            valueGwei: 5000000,
+            isContractCall: false,
+        };
+        const result = filterTransactions([native], {
+            hideSpoofedSymbols: true,
+            hideFraudContracts: true,
+            hideLowHolderTokens: true,
+            hideDustTransactions: true,
+            dustThresholdGwei: 100000,
+        });
+        expect(result.transactions).toEqual([native]);
+    });
+});
+
+describe("surface 2: the Send token selector", () => {
+    let select;
+
+    function render(tokenBalances) {
+        select = { innerHTML: "", children: [] };
+        select.appendChild = (child) => select.children.push(child);
+        globalThis.document = {
+            getElementById: (id) => (id === "send-token" ? select : null),
+            createElement: () => ({ value: "", textContent: "" }),
+        };
+        renderSendTokenSelect({
+            address: "0x" + "a".repeat(40),
+            tokenBalances,
+        });
+    }
+
+    beforeEach(() => {
+        state.fraudContracts = [];
+        state.hideLowHolderTokens = true;
+    });
+
+    test("a fake ETH token is not selectable", () => {
+        render([
+            {
+                address: FAKE_ETH_CONTRACT,
+                symbol: "ETH",
+                decimals: 18,
+                balance: "0.005",
+                holders: 900000,
+            },
+        ]);
+        expect(select.children).toEqual([]);
+    });
+
+    test("native ETH remains the always-present option", () => {
+        render([]);
+        expect(select.innerHTML).toBe('<option value="ETH">ETH</option>');
+    });
+});
+
+describe("surface 3: the balance list", () => {
+    function respondWith(items) {
+        debugFetch.mockImplementation(async () => ({
+            ok: true,
+            status: 200,
+            statusText: "OK",
+            json: async () => items,
+        }));
+    }
+
+    function fakeEthItem(overrides = {}) {
+        return {
+            value: "5000000000000000",
+            token: {
+                type: "ERC-20",
+                address_hash: FAKE_ETH_CONTRACT,
+                symbol: "ETH",
+                name: "Ethereum",
+                decimals: "18",
+                holders_count: "900000",
+                ...overrides,
+            },
+        };
+    }
+
+    beforeEach(() => {
+        debugFetch.mockReset();
+    });
+
+    // The bug in issue #235: this token cleared the balance list's own
+    // 1,000-holder floor and was listed as a holding named ETH.
+    test("a fake ETH token clearing the holder floor is filtered", async () => {
+        respondWith([fakeEthItem()]);
+        expect(await fetchTokenBalances(HOLDER, BLOCKSCOUT, [])).toEqual([]);
+    });
+
+    test("tracking the fake token manually does not admit it either", async () => {
+        respondWith([fakeEthItem({ holders_count: "0" })]);
+        const balances = await fetchTokenBalances(HOLDER, BLOCKSCOUT, [
+            { address: FAKE_ETH_CONTRACT },
+        ]);
+        expect(balances).toEqual([]);
+    });
+
+    test("a genuine token keeps its place in the list", async () => {
+        respondWith([
+            fakeEthItem({
+                address_hash: USDC_CONTRACT,
+                symbol: "USDC",
+                name: "USD Coin",
+                decimals: "6",
+            }),
+        ]);
+        const balances = await fetchTokenBalances(HOLDER, BLOCKSCOUT, []);
+        expect(balances).toHaveLength(1);
+        expect(balances[0].symbol).toBe("USDC");
+    });
+
+    // The trap in this change: the user's real ETH balance is not an ERC-20
+    // and is fetched over RPC in refreshBalances, so it never passes through
+    // this loop at all.  An explorer row that is not an ERC-20 is dropped
+    // before the symbol rule is consulted.
+    test("a non-ERC-20 row claiming ETH never reaches the symbol rule", async () => {
+        respondWith([fakeEthItem({ type: "ERC-721" })]);
+        expect(await fetchTokenBalances(HOLDER, BLOCKSCOUT, [])).toEqual([]);
+    });
+
+    // The money test: the user holds real ETH and has been airdropped a fake
+    // ETH ERC-20.  The fake is gone from the list of tokens; the real balance
+    // is exactly what the node reported.
+    test("the real native ETH balance survives a fake ETH airdrop", async () => {
+        respondWith([fakeEthItem()]);
+        const addr = { address: HOLDER };
+        await refreshBalances(
+            [{ addresses: [addr] }],
+            "https://rpc.example.invalid",
+            BLOCKSCOUT,
+            [],
+        );
+        expect(addr.balance).toBe("1.2345");
+        expect(addr.tokenBalances).toEqual([]);
+    });
+
+    test("no test in this file performed a network request", () => {
+        expect(global.fetch).not.toHaveBeenCalled();
+    });
+});

tests/walletDelete.test.js

@@ -1,4 +1,6 @@
 const {
+    canRemoveAddress,
+    removeAddressFromState,
     removeWalletFromState,
     broadcastActiveChanged,
 } = require("../src/shared/walletDelete");
@@ -6,6 +8,7 @@ const {
 // Fixed addresses — never used for anything but these tests.
 const A0 = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
 const A1 = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
+const A2 = "0x514910771AF9Ca656af840dff83E8264EcF986CA";
 const B0 = "0x2260FAC5E5542a773Aa44fBCfeDf7C193bc2C599";
 const C0 = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
 
@@ -111,6 +114,219 @@ describe("removeWalletFromState", () => {
     });
 });
 
+// An HD wallet with three addresses next to a single-address key wallet.
+// `nextIndex` is the wallet's derivation high-water mark, three addresses in.
+function makeAddressState(overrides = {}) {
+    return {
+        hasWallet: true,
+        wallets: [
+            { ...wallet("A", [A0, A1, A2]), type: "hd", nextIndex: 3 },
+            { ...wallet("B", [B0]), type: "key" },
+        ],
+        selectedWallet: 0,
+        selectedAddress: 0,
+        activeAddress: A0,
+        allowedSites: { [A0]: ["a.example"], [A1]: ["b.example"] },
+        deniedSites: { [A1]: ["d.example"], [B0]: ["e.example"] },
+        ...overrides,
+    };
+}
+
+describe("canRemoveAddress", () => {
+    test("an HD wallet with more than one address may remove one", () => {
+        expect(canRemoveAddress({ type: "hd", addresses: [{}, {}] })).toBe(
+            true,
+        );
+    });
+
+    test("an xprv wallet with more than one address may too", () => {
+        expect(canRemoveAddress({ type: "xprv", addresses: [{}, {}] })).toBe(
+            true,
+        );
+    });
+
+    // The last address is what delete-wallet is for.
+    test("a wallet holding a single address may not", () => {
+        expect(canRemoveAddress({ type: "hd", addresses: [{}] })).toBe(false);
+    });
+
+    // A key wallet holds one bare private key and cannot derive more, so it
+    // has no "+" button and gets no remove control either.
+    test("a key wallet may not, whatever its address count", () => {
+        expect(canRemoveAddress({ type: "key", addresses: [{}] })).toBe(false);
+        expect(canRemoveAddress({ type: "key", addresses: [{}, {}] })).toBe(
+            false,
+        );
+    });
+
+    test("a missing or typeless wallet may not", () => {
+        expect(canRemoveAddress(undefined)).toBe(false);
+        expect(canRemoveAddress({})).toBe(false);
+    });
+});
+
+describe("removeAddressFromState", () => {
+    test("removing a non-selected address leaves the selection where it is", () => {
+        const state = makeAddressState({
+            selectedAddress: 2,
+            activeAddress: A2,
+        });
+
+        const { removed, activeAddressChanged } = removeAddressFromState(
+            state,
+            0,
+            0,
+        );
+
+        expect(removed).toBe(true);
+        // A2 moved from index 2 to index 1 by the splice.
+        expect(state.wallets[0].addresses.map((a) => a.address)).toEqual([
+            A1,
+            A2,
+        ]);
+        expect(state.selectedWallet).toBe(0);
+        expect(state.selectedAddress).toBe(1);
+        expect(state.activeAddress).toBe(A2);
+        expect(activeAddressChanged).toBe(false);
+        // The wallet list itself is untouched.
+        expect(state.wallets).toHaveLength(2);
+        expect(state.hasWallet).toBe(true);
+    });
+
+    test("removing an address after the selection does not shift it", () => {
+        const state = makeAddressState({
+            selectedAddress: 0,
+            activeAddress: A0,
+        });
+
+        const { removed, activeAddressChanged } = removeAddressFromState(
+            state,
+            0,
+            2,
+        );
+
+        expect(removed).toBe(true);
+        expect(state.selectedAddress).toBe(0);
+        expect(state.activeAddress).toBe(A0);
+        expect(activeAddressChanged).toBe(false);
+    });
+
+    test("a selection in another wallet is untouched", () => {
+        const state = makeAddressState({
+            selectedWallet: 1,
+            selectedAddress: 0,
+            activeAddress: B0,
+        });
+
+        const { removed, activeAddressChanged } = removeAddressFromState(
+            state,
+            0,
+            1,
+        );
+
+        expect(removed).toBe(true);
+        expect(state.selectedWallet).toBe(1);
+        expect(state.selectedAddress).toBe(0);
+        expect(state.activeAddress).toBe(B0);
+        expect(activeAddressChanged).toBe(false);
+    });
+
+    test("removing the selected address falls back to the wallet's first address", () => {
+        const state = makeAddressState({
+            selectedAddress: 1,
+            activeAddress: A1,
+        });
+
+        const { removed, activeAddressChanged } = removeAddressFromState(
+            state,
+            0,
+            1,
+        );
+
+        expect(removed).toBe(true);
+        expect(state.wallets[0].addresses.map((a) => a.address)).toEqual([
+            A0,
+            A2,
+        ]);
+        expect(state.selectedWallet).toBe(0);
+        expect(state.selectedAddress).toBe(0);
+        expect(state.activeAddress).toBe(A0);
+        expect(activeAddressChanged).toBe(true);
+    });
+
+    // The active address can be persisted in a different case than the
+    // wallet's copy of it, so the comparison must not be literal.
+    test("the active address is matched case-insensitively", () => {
+        const state = makeAddressState({
+            selectedAddress: 1,
+            activeAddress: A1.toLowerCase(),
+        });
+
+        const { activeAddressChanged } = removeAddressFromState(state, 0, 1);
+
+        expect(state.activeAddress).toBe(A0);
+        expect(activeAddressChanged).toBe(true);
+    });
+
+    test("site permissions are dropped for the removed address only", () => {
+        const state = makeAddressState();
+
+        removeAddressFromState(state, 0, 1);
+
+        expect(state.allowedSites).toEqual({ [A0]: ["a.example"] });
+        expect(state.deniedSites).toEqual({ [B0]: ["e.example"] });
+    });
+
+    // The derivation counter is a high-water mark, never rewound: "+" derives
+    // a fresh index rather than re-deriving the address just removed.
+    test("the wallet's derivation counter is not rewound", () => {
+        const state = makeAddressState();
+
+        removeAddressFromState(state, 0, 1);
+
+        expect(state.wallets[0].nextIndex).toBe(3);
+    });
+
+    test("the last address of a wallet is refused, and nothing changes", () => {
+        const state = makeAddressState({
+            selectedWallet: 1,
+            selectedAddress: 0,
+            activeAddress: B0,
+        });
+
+        const { removed, activeAddressChanged } = removeAddressFromState(
+            state,
+            1,
+            0,
+        );
+
+        expect(removed).toBe(false);
+        expect(activeAddressChanged).toBe(false);
+        expect(state.wallets[1].addresses.map((a) => a.address)).toEqual([B0]);
+        expect(state.activeAddress).toBe(B0);
+        expect(state.hasWallet).toBe(true);
+    });
+
+    // The same refusal reached the other way: an HD wallet worn down to one
+    // address is no more removable than a key wallet.
+    test("an HD wallet down to its last address is refused too", () => {
+        const state = makeAddressState();
+
+        expect(removeAddressFromState(state, 0, 2).removed).toBe(true);
+        expect(removeAddressFromState(state, 0, 1).removed).toBe(true);
+        expect(removeAddressFromState(state, 0, 0).removed).toBe(false);
+        expect(state.wallets[0].addresses.map((a) => a.address)).toEqual([A0]);
+    });
+
+    test("an out-of-range address index is refused", () => {
+        const state = makeAddressState();
+
+        expect(removeAddressFromState(state, 0, 7).removed).toBe(false);
+        expect(removeAddressFromState(state, 7, 0).removed).toBe(false);
+        expect(state.wallets[0].addresses).toHaveLength(3);
+    });
+});
+
 describe("broadcastActiveChanged", () => {
     afterEach(() => {
         delete global.chrome;