Compare commits

...

4 Commits

Author SHA1 Message Date
5d8d18f9cd test: drive the Settings screen in a browser and guard every popup element id (closes #229)
Some checks failed
e2e / e2e-chrome (push) Has been cancelled
e2e / e2e-firefox (push) Has been cancelled
check / check (push) Has been cancelled
Nothing exercised the Settings view in a browser, and jest runs in the
node environment with no DOM, so the densest run of $("...") lookups in
the codebase was unverified at runtime. A wrong id is valid JavaScript
naming a defined function: $() returns null and the next property access
throws, which inside a view's init() aborts the rest of the popup's
init() and leaves every screen blank.

Two halves, because they catch different things.

The e2e suite (tests/e2e/run.js) gains seven cases between the address
removal and dust threshold sections. They assert the About well and the
wallet list were actually written — show() populates those near its end,
only the debug well and the debug-mode checkbox follow, so reading them
back proves show() ran through to there rather than just far enough to
unhide the section — that the four Token Spam Protection controls are
real input[type=checkbox] elements defaulted on, and that the theme and
network selectors offer exactly the choices src/shared/networks.js and
index.html define.

What the selectors persist is asserted by a round trip through
NON-DEFAULT values: they are driven to dark and sepolia, the popup is
closed and reopened, both are read back, and both are then restored the
same way and reasserted after a second reopen. Neither value is the
first <option> of its <select>, which is the point — the first option is
what the DOM reports with no JavaScript having run at all, so asserting
it would pass just as happily against a Settings screen that assigned
nothing. One spam filter is likewise toggled off and back on across a
reopen each way. Those round trips run the change handler, saveState(),
loadState() and the assignments show() and init() make, rather than only
looking at the screen. Each group records a coverage key and a final
case demands the exact set, so a section that silently stopped running
reddens the suite instead of shrinking it.

show() no longer wraps its settings-network lookup in if (networkSelect),
and neither does init(): a null there was silently skipped, which is
exactly the failure this change exists to make loud.

tests/popupElementIds.test.js is the general half and needs no browser,
so jest picks it up and it runs in make check: every literal id reached
through $(), document.getElementById(), showError()/hideError() and
showView() must exist in src/popup/index.html, no id in index.html may
be defined twice, and the scan asserts it found the code and the markup
so it cannot pass by covering nothing. Only literal arguments are
resolvable statically; $(containerId) and a lookup naming the wrong
existing element are the browser suites' job, and README says so.

Demonstrated against four deliberate breaks. A typo'd id in settings.js
reddens both halves, the e2e run reporting "pageerror: Cannot set
properties of null (setting 'checked')" against its first test. A
handler bound to the wrong but existing element passes the static guard
and reddens only the new functional case. A typo in a view no browser
suite opens reddens only the static guard. Deleting either persisted
value assignment in settings.js — the theme one in init(), the network
one in show() — reddens the selector round trip and nothing else, each
one on its own.
2026-08-17 06:58:41 +00:00
743b1962a5 build: run the browser e2e suites in CI (closes #259)
All checks were successful
check / check (push) Successful in 28s
e2e / e2e-chrome (push) Successful in 47s
e2e / e2e-firefox (push) Successful in 20s
2026-08-17 08:52:26 +02:00
c06765ef8f fix: one transaction approval at a time, and honest copy for a nonce collision (closes #271)
All checks were successful
check / check (push) Successful in 28s
2026-08-17 08:38:26 +02:00
e07efb710a fix: an address holding only unpriced tokens is no longer totalled at $0.00 (closes #261)
Some checks failed
check / check (push) Has been cancelled
2026-08-17 08:38:10 +02:00
22 changed files with 2061 additions and 214 deletions

49
.gitea/workflows/e2e.yml Normal file
View File

@@ -0,0 +1,49 @@
name: e2e
on: [push]
# The browser end-to-end suites, one job per browser, deliberately kept out
# of the check workflow: REPO_POLICIES.md caps make test at 20 seconds and
# script/cibuild is a plain `docker build .` whose Dockerfile runs
# make check, so folding a browser suite into either would blow that cap
# and slow the local fast path. Before this workflow every browser-level
# guarantee in this repo held only when a human remembered to run it.
#
# One job per browser rather than two steps in one job, so a Chrome failure
# does not hide the Firefox result.
#
# Each job is one script and nothing else. Both scripts need docker and
# nothing else — they deliver the repo to the daemon as a build context and
# build the extension inside the pinned image — which is what makes them
# runnable here at all: the runner executes the job in a container against
# the host's docker socket, so a `-v "$PWD:/work"` source path is resolved
# by the host daemon and mounts an empty directory, and the runner image's
# node is too old to install this repo's dependencies.
#
# These jobs REPORT, they do not gate. Whether a check blocks a merge is
# Gitea branch protection, which this repo does not configure, so a failure
# here is a red mark a reviewer has to account for rather than a hard
# block. Making e2e-chrome a required check is blocked on the measured
# flake in the dApp signing wait -- two of six runs of unmutated code on a
# loaded machine -- tracked as
# https://git.eeqj.de/sneak/AutistMask/issues/287. A gate that fails at
# random teaches people to merge past red.
#
# Nothing here may pass vacuously. There is no continue-on-error and no
# `|| true`. Both scripts exit non-zero when docker is missing, when the
# image build fails, and when the browser fails to start; the Chrome
# harness aborts the suite outright if its network interception is not in
# effect.
jobs:
e2e-chrome:
runs-on: ubuntu-latest
steps:
# actions/checkout v4.2.2, 2026-02-22
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
- run: script/test-e2e
e2e-firefox:
runs-on: ubuntu-latest
steps:
# actions/checkout v4.2.2, 2026-02-22
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
- run: script/test-e2e-firefox

155
README.md
View File

@@ -83,10 +83,11 @@ provide:
git pre-commit hook git pre-commit hook
- `script/projectname` — print the project name (used for the Docker image tag) - `script/projectname` — print the project name (used for the Docker image tag)
- `script/test` — run the test suite (jest) - `script/test` — run the test suite (jest)
- `script/test-e2e` — run the Chrome browser end-to-end suite (docker required; - `script/test-e2e` — run the Chrome browser end-to-end suite (docker is the
see [End-to-End Tests](#end-to-end-tests)) only prerequisite: it builds a pinned image that carries the repo and a fresh
- `script/test-e2e-firefox` — run the Firefox browser end-to-end suite (docker extension build, see [End-to-End Tests](#end-to-end-tests))
required; builds its own pinned image, see - `script/test-e2e-firefox` — run the Firefox browser end-to-end suite (same,
against an image with a pinned Firefox and geckodriver, see
[End-to-End Tests](#end-to-end-tests)) [End-to-End Tests](#end-to-end-tests))
- `script/lint` — run the linter - `script/lint` — run the linter
- `script/fmt` — format all files (writes) - `script/fmt` — format all files (writes)
@@ -136,11 +137,12 @@ are outside `make check`.
`make test-e2e` builds `dist/chrome/` and drives the **real popup in a real `make test-e2e` builds `dist/chrome/` and drives the **real popup in a real
Chrome**, loaded as an unpacked MV3 extension inside a pinned Chrome**, loaded as an unpacked MV3 extension inside a pinned
`mcr.microsoft.com/playwright` container (pinned by digest in `script/test-e2e`; `mcr.microsoft.com/playwright` container (pinned by digest in
docker is required and the suite fails loudly rather than skipping if it is `tests/e2e/Dockerfile`, which is also where the extension is built; docker is
unavailable). The suite lives in `tests/e2e/` and is driven by required and the suite fails loudly rather than skipping if it is unavailable).
`playwright-core`, whose version must stay matched to the container's Playwright The suite lives in `tests/e2e/` and is driven by `playwright-core`, whose
version — the browsers ship inside the image. version must stay matched to the container's Playwright version — the browsers
ship inside the image.
It covers popup load, WebAssembly compilation under the shipped CSP (see It covers popup load, WebAssembly compilation under the shipped CSP (see
[Content Security Policy](#content-security-policy)), wallet creation through [Content Security Policy](#content-security-policy)), wallet creation through
@@ -157,6 +159,23 @@ fixtures in `tests/e2e/network.js`, so the run is deterministic and fully
offline; unrecognised outbound requests are reported as failures rather than offline; unrecognised outbound requests are reported as failures rather than
silently allowed. silently allowed.
It also covers the **Settings screen**, which holds the densest run of element
id lookups in the codebase and where one wrong id leaves the whole popup blank
rather than only degrading Settings: that the screen renders populated — the
About well and the wallet list are read back, so a `show()` that stopped early
is caught rather than merely a view that failed to appear — that the four Token
Spam Protection controls are real checkboxes defaulted on, and that the theme
and network selectors offer exactly the choices `src/shared/networks.js` and
`index.html` define. What the selectors persist is then driven through the UI to
`dark` and `sepolia` and reasserted after a popup reopen before being restored
the same way; neither value is its selector's first `<option>`, so neither can
be read back from the markup with no JavaScript having run. One spam filter is
likewise toggled off and back on across a reopen each way, which exercises the
change handler, `saveState()`, `loadState()` and the assignment `init()` makes.
Each group of these assertions records a coverage key and a final case demands
the exact set, so a section that silently stopped running reddens the suite
instead of quietly shrinking it.
It also covers the confirmation screen, for both a native ETH send and an ERC-20 It also covers the confirmation screen, for both a native ETH send and an ERC-20
send: Send disabled while the fee estimate is in flight, enabled once it lands, send: Send disabled while the fee estimate is in flight, enabled once it lands,
the fee block quoting the expected cost and the reserve separately, the distinct the fee block quoting the expected cost and the reserve separately, the distinct
@@ -245,11 +264,13 @@ 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` it is a small WebDriver client built on global `fetch` and `child_process`
against geckodriver's HTTP API. against geckodriver's HTTP API.
Unlike the Chrome suite it builds its own container image rather than pulling a Both suites build their own image, each with the repo and a fresh extension
published one, because no published image carries both a pinned Firefox and a build baked in; what differs is the base. The Chrome image layers those on top
matching geckodriver. `tests/e2e/firefox/Dockerfile` pins all three external of a published Playwright image, whereas this one is assembled from a `node`
artifacts by digest — the `node` base image, the Firefox 153.0.3 tarball, and base, because no published image carries both a pinned Firefox and a matching
geckodriver 0.36.0 — and the Firefox version in particular must not float: 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. `-remote-allow-system-access` is **mandatory** on 153 and was not on 142.
Without that flag, both navigating to `moz-extension://` and running Without that flag, both navigating to `moz-extension://` and running
chrome-context script fail with `unsupported operation`. The flag grants the chrome-context script fail with `unsupported operation`. The flag grants the
@@ -317,9 +338,64 @@ Two limits are worth knowing, both real differences from the Chrome suite:
Neither `make test-e2e` nor `make test-e2e-firefox` is part of `make check` or 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 `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 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 — pick it up either. Run them locally before changing anything under
docker-in-docker in CI is a separate question. Run them locally before changing `src/popup/views/`.
anything under `src/popup/views/`.
### In CI
`.gitea/workflows/e2e.yml` runs both suites on every push, as two jobs —
`e2e-chrome` and `e2e-firefox` — separate from the `check` workflow, so the
20-second `make test` cap and the local fast path are untouched. Each job is a
checkout and the matching `script/` entrypoint, nothing else.
Docker is the only thing either job needs from the runner, and that is not an
accident. The runner executes a job inside a container against the **host's**
docker daemon, so a `docker run -v "$PWD:/work"` source path is resolved by the
host and mounts an empty directory, and the runner image's node is too old to
install this repo's dependencies. Both suites therefore ship the repo to the
daemon as a build context and build the extension inside the image, which works
identically on a laptop.
The jobs **report, they do not gate.** A failure is a red mark against the
commit that a reviewer has to account for, not a hard block: whether a check
blocks a merge is Gitea branch protection, which this repo does not configure.
That is not only a statement about configuration. The Chrome suite is
**measurably flaky under load** — two of six runs of unmutated code on a busy
machine lost the approval popup out from under the dApp signing wait, always in
the `#183` section, tracked as
[#287](https://git.eeqj.de/sneak/AutistMask/issues/287). So a red `e2e-chrome`
has to be read before it is believed, and that flake is the blocker to ever
making this a required check. Do not answer it with a retry wrapper: a suite
that reruns until it is green stops being evidence.
Nothing in either job can pass vacuously. There is no `continue-on-error` and no
`|| true`; both scripts exit non-zero when docker is missing, when the image
build fails, and when the browser fails to start; the Chrome harness aborts the
suite outright if its network interception is not in effect.
Measured on this repo's runner: `e2e-chrome` about 1m55s cold, almost all of it
the one-time pull of the pinned ~800MB Playwright layer, and well under a minute
once that layer is cached. `e2e-firefox` about 1m05s cold, and it caches its
Firefox and geckodriver downloads the same way.
### Element id guard (part of `make check`)
`tests/popupElementIds.test.js` asserts statically that every element id the
popup looks up — `$("...")`, `document.getElementById("...")`,
`showError()`/`hideError()`, and the `view-<name>` a literal `showView("...")`
resolves to — exists in `src/popup/index.html`, and that `index.html` defines no
id twice. A wrong id is valid JavaScript naming a defined function, so neither
jest (node environment, no DOM) nor a linter objects to it; at runtime `$()`
returns `null` and the next property access throws, which inside a view's
`init()` aborts the rest of `src/popup/index.js` `init()` and leaves the popup
blank.
It runs with no browser, so unlike the e2e suites it fits inside `make check`,
and it covers every view rather than the ones some test happens to open. It only
sees literal arguments: a call like `$(containerId)` is invisible to it, and a
lookup naming the wrong existing element is valid by construction. Both of those
are the browser suites' job.
## Rationale ## Rationale
@@ -538,6 +614,27 @@ Both are click-copyable. Truncating to 4 decimals in summary views is acceptable
for scannability, but the detail view must never discard precision — it is the for scannability, but the detail view must never discard precision — it is the
one place the user can always use to verify exact details. one place the user can always use to verify exact details.
#### Partial USD totals
Prices are fetched for the top 25 tokens only, so an address can hold assets the
extension has no price for. Worth zero and worth an unknown amount are different
facts and are never collapsed into one number. `getAddressValue()` in
`src/shared/prices.js` returns `{ usd, partial }` — the value of the priced
holdings, and whether an unpriced holding was left out of it — and every screen
renders it through `formatAddressTotal()`, so the wording cannot drift:
- Nothing knowable (testnet, or before the first price fetch): no total line.
- Everything priced: `Total: $5,500.00`.
- Part priced: `Total: $3,000.00 plus unpriced tokens` — the figure is real as
far as it goes and is kept, named as a floor rather than the total.
- Nothing priced but something held: `Total: unpriced tokens only`. No figure,
because the only figure available would be the `$0.00` sum of an empty set,
and on the address-removal confirmation that sits directly under "This address
holds a balance."
The per-token balance lines are unaffected: each shows its quantity, and a USD
column that is blank for a token with no price.
#### Language & Labeling #### Language & Labeling
All user-facing text avoids unnecessary jargon wherever possible: All user-facing text avoids unnecessary jargon wherever possible:
@@ -673,7 +770,9 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
- **When**: At least one wallet exists. This is the root screen. - **When**: At least one wallet exists. This is the root screen.
- **Elements**: - **Elements**:
- Active address ETH balance (large) + USD value in parentheses - Active address ETH balance (large) + USD value in parentheses
- "Total:" USD value across ETH and every token shown for the active address - "Total:" USD value across ETH and every token shown for the active
address, written by `formatAddressTotal()` — see
[Partial USD totals](#partial-usd-totals)
- Active address (color dot, full address, etherscan link, tap to copy) - Active address (color dot, full address, etherscan link, tap to copy)
- Send / Receive quick-action buttons, both acting on the active address - Send / Receive quick-action buttons, both acting on the active address
- ETH/USD price display - ETH/USD price display
@@ -735,7 +834,7 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
- Title: "Wallet Name — Address N" - Title: "Wallet Name — Address N"
- ENS name (if resolved, bold above the address) - ENS name (if resolved, bold above the address)
- Full address (color dot, etherscan link, tap to copy) - Full address (color dot, etherscan link, tap to copy)
- USD total for address - USD total for address (see [Partial USD totals](#partial-usd-totals))
- Balance list: ETH + the ERC-20 tokens shown for this address (4 decimal - Balance list: ETH + the ERC-20 tokens shown for this address (4 decimal
places, USD inline). Each balance row is clickable → **AddressToken** places, USD inline). Each balance row is clickable → **AddressToken**
- Send / Receive / + Token buttons and a "···" menu button - Send / Receive / + Token buttons and a "···" menu button
@@ -1101,11 +1200,13 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
xprv wallet has no recovery phrase to re-import. xprv wallet has no recovery phrase to re-import.
- A warning when the address holds anything, ETH or any tracked ERC-20, - A warning when the address holds anything, ETH or any tracked ERC-20,
followed by the holdings themselves via `balanceLinesForAddress()` and the followed by the holdings themselves via `balanceLinesForAddress()` and the
USD total via `getAddressValueUsd()`. The sentence names no figure of its USD total via `formatAddressTotal()` (see
own: the lines round to four decimals, so a sentence built from a rounded [Partial USD totals](#partial-usd-totals)). The sentence names no figure
number would report `0.0000 ETH` for an address holding real money. The of its own: the lines round to four decimals, so a sentence built from a
predicate is `addressHoldsFunds()` in `src/popup/views/helpers.js`, rounded number would report `0.0000 ETH` for an address holding real
unrounded and token-aware. A balance is a warning, never a refusal. 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 - The rule that a wallet always keeps at least one address, and that
removing the last one means deleting the wallet from Settings removing the last one means deleting the wallet from Settings
- Error line - Error line
@@ -1175,7 +1276,11 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
opening the window, so the screen shows a complete transaction and the signed opening the window, so the screen shows a complete transaction and the signed
artifact can be compared with it field for field. A request that cannot be artifact can be compared with it field for field. A request that cannot be
populated — unreachable node, reverting gas estimate — opens no window and is populated — unreachable node, reverting gas estimate — opens no window and is
failed back to the site. failed back to the site. Only one transaction approval exists at a time:
populating fixes the nonce, so a second `eth_sendTransaction` arriving while
one is unanswered is refused with EIP-1193 code `-32002` rather than being
populated at the same nonce. It opens no window and takes no nonce, and the
site can send it again once the pending one is answered.
- **Elements**: - **Elements**:
- "Transaction Request" heading - "Transaction Request" heading
- Phishing warning banner (shown when the hostname is on the phishing - Phishing warning banner (shown when the hostname is on the phishing

94
TODO.md
View File

@@ -33,7 +33,8 @@ The backlog lives on the
authoritative; this file does not duplicate it. Full policy file set present. authoritative; this file does not duplicate it. Full policy file set present.
Real-browser end-to-end suites (`make test-e2e` for Chrome, Real-browser end-to-end suites (`make test-e2e` for Chrome,
`make test-e2e-firefox` for Firefox) now sit alongside `make check`, which `make test-e2e-firefox` for Firefox) now sit alongside `make check`, which
cannot see a runtime `ReferenceError` in a popup view. cannot see a runtime `ReferenceError` in a popup view, and
`.gitea/workflows/e2e.yml` runs both of them on every push.
# Next Step # Next Step
@@ -45,6 +46,50 @@ undefined identifiers, which is how
# Completed Steps # Completed Steps
- 2026-08-17: The Settings screen is driven in a browser, and every element id
the popup looks up is checked statically. Nothing exercised Settings in the
e2e suite, and jest runs with no DOM, so the densest run of `$("...")` lookups
in the codebase was unverified at runtime. Seven new cases in
`tests/e2e/run.js` reach Settings, assert the About well and the wallet list
were actually written, assert the four Token Spam Protection checkboxes are
real checkboxes defaulted on, and assert the theme and network selectors offer
the choices `src/shared/networks.js` and `index.html` define. The selectors
are then driven to `dark` and `sepolia` — neither is the first `<option>`, so
neither can be read back from the markup with no JavaScript involved — and
reasserted after a popup reopen before being restored the same way, and one
spam filter is toggled off and back on across a reopen each way. Those round
trips run the change handler, `saveState()`, `loadState()` and the
`init()`/`show()` assignments rather than just looking at the screen. `show()`
no longer guards its `settings-network` lookup with `if (networkSelect)`: a
missing element must fail loudly, which is the whole failure mode this unit
exists to catch. Each group records a coverage key and a final case demands
the exact set, so a shortened or skipped section reddens the run instead of
shrinking it. `tests/popupElementIds.test.js` is the general half and runs in
`make check` with no browser: every literal id reached through `$()`,
`document.getElementById()`, `showError()`/`hideError()` and `showView()` must
exist in `src/popup/index.html`, which no id in `index.html` may define twice.
Demonstrated on four deliberate breaks — a typo'd id (both halves red), a
handler bound to the wrong but existing element (only the functional e2e case
red), a typo in a view no browser suite opens (only the static guard red), and
the deletion of both persisted-value assignments in `settings.js` (only the
selector round-trip case red)
([#229](https://git.eeqj.de/sneak/AutistMask/issues/229)).
- 2026-08-17: An address total no longer reports `$0.00` for holdings it cannot
price. Prices exist for the top 25 tokens only, so the priced-only sum was
printed as the total and an address holding nothing but unpriced ERC-20s was
shown as worth nothing — directly under "This address holds a balance." on the
address-removal confirmation. `getAddressValue()` in `src/shared/prices.js`
now returns `{ usd, partial }`, keeping worth-zero and worth-an-unknown-amount
apart the way an absent `holders_count` is kept apart from a count of zero,
and every screen renders it through the one `formatAddressTotal()`: the figure
when it covers everything, the figure marked `plus unpriced tokens` when it
covers part, and `Total: unpriced tokens only` when it would cover nothing.
Home, AddressDetail and the removal confirmation all read it, and
`getWalletValue()`/`getTotalValue()` carry `partial` up. Covered by
`tests/addressValue.test.js` — the only-unpriced, genuinely-zero and
fully-priced cases at the helper and at both call sites that return their
markup — demonstrated failing first
([#261](https://git.eeqj.de/sneak/AutistMask/issues/261)).
- 2026-08-17: `README.md` no longer advertises a defect the wallet does not - 2026-08-17: `README.md` no longer advertises a defect the wallet does not
have. The End-to-End Tests section listed the EIP-1193 code being dropped in have. The End-to-End Tests section listed the EIP-1193 code being dropped in
the last hop into the page as a standing limit of the dApp coverage; that the last hop into the page as a standing limit of the dApp coverage; that
@@ -54,6 +99,31 @@ undefined identifiers, which is how
stubbed RPC and the unobservable toolbar popup — were checked against the stubbed RPC and the unobservable toolbar popup — were checked against the
current `src/content/inpage.js` and `tests/e2e/` and left as they are current `src/content/inpage.js` and `tests/e2e/` and left as they are
([#285](https://git.eeqj.de/sneak/AutistMask/issues/285)). ([#285](https://git.eeqj.de/sneak/AutistMask/issues/285)).
- 2026-08-17: One transaction approval at a time. Populating in the background
before the window opens is what makes the displayed object the verified
object, and it also fixes the nonce: two `eth_sendTransaction` calls populated
concurrently took the same nonce from a node that had seen neither broadcast,
and the second could then never be sent, because the only way to give it a
fresh nonce is to populate it again after the user has read the old one off
the screen. A second request is now refused with EIP-1193 `-32002` while one
is unanswered — the slot is taken immediately before population, after the
authorization checks, so no second nonce is allocated, no second window opens,
and a page the wallet refuses anyway cannot hold the slot against the
connected site. The slot is freed at `settleApproval()`, the single point an
approval is retired, so every path that ends an approval ends the hold with
it; an approval whose window is gone and whose attempt has failed is settled
there rather than left waiting on a window that no longer exists, and an
approval window that could not be opened at all is answered with `-32603`
instead of holding the page's promise open. Signature approvals are not gated,
consuming no nonce. A collision that does happen is also reported accurately
now: a broadcast the node refused for the nonce, and an approval carrying a
nonce this worker has already broadcast for that address on that chain (caught
before the node is asked at all), both say the transaction did not reach the
network and to send it again, instead of warning that it may have sent. The
record is keyed by chain as well as address, because nonce spaces are per
chain and low nonces overlap across them. `already known` deliberately keeps
the ambiguous wording, because a node that says it has the transaction has it
([#271](https://git.eeqj.de/sneak/AutistMask/issues/271)).
- 2026-08-14: The parts of the - 2026-08-14: The parts of the
[#150](https://git.eeqj.de/sneak/AutistMask/issues/150) and [#150](https://git.eeqj.de/sneak/AutistMask/issues/150) and
[#151](https://git.eeqj.de/sneak/AutistMask/issues/151) definition of done the [#151](https://git.eeqj.de/sneak/AutistMask/issues/151) definition of done the
@@ -70,6 +140,24 @@ undefined identifiers, which is how
on the real clipboard, read back after a sentinel write. Each of the four was on the real clipboard, read back after a sentinel write. Each of the four was
demonstrated failing against a deliberately broken build demonstrated failing against a deliberately broken build
([#188](https://git.eeqj.de/sneak/AutistMask/issues/188)). ([#188](https://git.eeqj.de/sneak/AutistMask/issues/188)).
- 2026-08-14: CI runs the browser end-to-end suites. `.gitea/workflows/e2e.yml`
runs `script/test-e2e` and `script/test-e2e-firefox` as two jobs on every
push, separate from `check`, so `make check` and its 20-second `make test` cap
are untouched. Every browser-level guarantee in this repo — the WASM-under-CSP
check, the recovery-phrase and private-key DOM wipes, the ConfirmTx spend
gate, the dApp approval round trips — was enforced only when a human
remembered to run it by hand. The suites could not run on the runner as they
stood: the runner executes a job in a container against the host's docker
daemon, so `docker run -v "$PWD:/work"` mounts an empty directory (measured),
and the runner image's node cannot install this repo's dependencies. Both
suites now ship the repo to the daemon as a build context and build the
extension inside the pinned image, so docker is the only prerequisite on a
runner or a laptop, and both run the image by ID rather than by tag so
concurrent clones cannot swap it. The jobs report rather than gate — this repo
configures no branch protection, and the Chrome suite is measurably flaky
under load, filed as [#287](https://git.eeqj.de/sneak/AutistMask/issues/287)
rather than papered over
([#259](https://git.eeqj.de/sneak/AutistMask/issues/259)).
- 2026-08-12: EIP-1193 error codes now reach the page. `src/content/inpage.js` - 2026-08-12: EIP-1193 error codes now reach the page. `src/content/inpage.js`
rebuilt every failure as `new Error(error.message)`, so the code the rebuilt every failure as `new Error(error.message)`, so the code the
background produced and the content script relayed intact was dropped in the background produced and the content script relayed intact was dropped in the
@@ -393,9 +481,5 @@ tracker.
- Pre-1.0 security review of the extension (key handling, DEBUG mode policy, RPC - 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 input validation) before any 1.0rc tag. Individual filed issues are parts of
it, but the review is broader than any of them. it, but the review is broader than any of them.
- Decide whether docker-in-docker makes `make test-e2e` and
`make test-e2e-firefox` runnable in the Gitea workflow. Extending the Chrome
suite itself is tracked as
[#183](https://git.eeqj.de/sneak/AutistMask/issues/183).
- Cut 1.0.0 once the milestone is empty, then continue tagging as milestones - Cut 1.0.0 once the milestone is empty, then continue tagging as milestones
land. land.

View File

@@ -7,17 +7,29 @@
# caps make test at 20 seconds and a browser suite does not fit. Run it # caps make test at 20 seconds and a browser suite does not fit. Run it
# yourself before touching popup views; it is the only check that can see # yourself before touching popup views; it is the only check that can see
# a used-but-not-imported identifier blow up at runtime. # a used-but-not-imported identifier blow up at runtime.
# .gitea/workflows/e2e.yml also runs it on every push, in a job separate
# from check so that cap and the local fast path both stay intact.
#
# Docker is the only prerequisite. The repo reaches the container as a
# build context and the extension is built inside it (see
# tests/e2e/Dockerfile), so nothing here depends on the node, yarn or make
# on the machine that starts the run. That is not a convenience: a bind
# mount cannot work under Gitea Actions, and the runner image's node is too
# old to install this repo's dependencies.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
# mcr.microsoft.com/playwright:v1.56.0-noble, 2026-08-09 IMAGE="$("$SCRIPT_DIR/projectname")-e2e-chrome"
#
# The playwright-core devDependency is pinned to the matching Playwright IIDFILE=""
# version (1.56.0) and the two must be bumped together: the browsers ship
# inside this image, and playwright-core looks for the exact browser cleanup() {
# revision its own version expects. A mismatch fails at launch. if [ -n "$IIDFILE" ]; then
IMAGE="mcr.microsoft.com/playwright@sha256:35246d87a7c88ea9b771c65d33171b2611b02a8253b4b12ce6f94376c55f99f2" rm -f "$IIDFILE"
fi
}
main() { main() {
cd "$ROOT" cd "$ROOT"
@@ -27,14 +39,23 @@ main() {
exit 1 exit 1
fi fi
echo "Building extension for e2e..." IIDFILE="$(mktemp)"
yarn run build 2>&1 trap cleanup EXIT
trap 'cleanup; exit 130' INT TERM
echo "Building the Chrome e2e image (extension included)..."
docker build --iidfile "$IIDFILE" -t "$IMAGE" -f tests/e2e/Dockerfile .
echo "Running e2e suite in the pinned Playwright container..." echo "Running e2e suite in the pinned Playwright container..."
# The image is run by ID, not by tag: where two clones of this repo run
# the suite at once, the other build can move the tag between this
# build and this run, and the suite would then silently test the other
# checkout.
#
# --ipc=host: Chromium's shared-memory needs more than the default # --ipc=host: Chromium's shared-memory needs more than the default
# 64MB /dev/shm or renderers crash. # 64MB /dev/shm or renderers crash.
# --user: keep files the suite touches owned by the caller, not root. # HOME=/tmp: the image's root home is not a reliable place for the
# HOME=/tmp: the mapped uid has no home directory in the image. # browser profile.
# PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1: without it, # PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1: without it,
# ctx.route() intercepts page requests only, and every fetch made by # ctx.route() intercepts page requests only, and every fetch made by
# the MV3 background service worker — including the phishing # the MV3 background service worker — including the phishing
@@ -51,13 +72,10 @@ main() {
# on a deliberate bump. # on a deliberate bump.
docker run --rm \ docker run --rm \
--ipc=host \ --ipc=host \
--user "$(id -u):$(id -g)" \
-e HOME=/tmp \ -e HOME=/tmp \
-e PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1 \ -e PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1 \
-e "E2E_TRACE_NETWORK=${E2E_TRACE_NETWORK:-0}" \ -e "E2E_TRACE_NETWORK=${E2E_TRACE_NETWORK:-0}" \
-v "$ROOT:/work" \ "$(cat "$IIDFILE")" \
-w /work \
"$IMAGE" \
node tests/e2e/run.js node tests/e2e/run.js
} }

View File

@@ -5,12 +5,17 @@
# #
# Deliberately NOT called by script/check or script/test, for the same # 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 # reason as the Chrome suite: REPO_POLICIES.md caps make test at 20 seconds
# and a browser suite does not fit. # and a browser suite does not fit. .gitea/workflows/e2e.yml also runs it
# on every push, in a job separate from check.
# #
# Unlike script/test-e2e this builds its image locally, because no # Unlike script/test-e2e this builds its base image locally, because no
# published image carries both a pinned Firefox and a matching geckodriver. # published image carries both a pinned Firefox and a matching geckodriver.
# All three external artifacts are pinned by digest inside the Dockerfile; # All three external artifacts are pinned by digest inside the Dockerfile;
# see tests/e2e/firefox/Dockerfile. # see tests/e2e/firefox/Dockerfile, which also explains why the repo and
# the extension build are baked into the image rather than mounted.
#
# Docker is the only prerequisite: nothing here depends on the node, yarn
# or make on the machine that starts the run.
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
@@ -18,6 +23,14 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
IMAGE="$("$SCRIPT_DIR/projectname")-e2e-firefox" IMAGE="$("$SCRIPT_DIR/projectname")-e2e-firefox"
IIDFILE=""
cleanup() {
if [ -n "$IIDFILE" ]; then
rm -f "$IIDFILE"
fi
}
main() { main() {
cd "$ROOT" cd "$ROOT"
@@ -26,16 +39,20 @@ main() {
exit 1 exit 1
fi fi
echo "Building extension for e2e..." IIDFILE="$(mktemp)"
yarn run build 2>&1 trap cleanup EXIT
trap 'cleanup; exit 130' INT TERM
# The build context is tests/e2e/firefox/ and holds nothing but the echo "Building the pinned Firefox e2e image (extension included)..."
# Dockerfile: the harness itself arrives over the bind mount below, so docker build --iidfile "$IIDFILE" -t "$IMAGE" \
# editing it never invalidates an image layer. -f tests/e2e/firefox/Dockerfile .
echo "Building the pinned Firefox e2e image..."
docker build -t "$IMAGE" "$ROOT/tests/e2e/firefox"
echo "Running the Firefox e2e suite..." echo "Running the Firefox e2e suite..."
# The image is run by ID, not by tag: where two clones of this repo run
# the suite at once, the other build can move the tag between this
# build and this run, and the suite would then silently test the other
# checkout.
#
# --shm-size=1g: Firefox needs more than the default 64MB /dev/shm. # --shm-size=1g: Firefox needs more than the default 64MB /dev/shm.
# --network none: the suite stubs nothing, so this is what keeps the # --network none: the suite stubs nothing, so this is what keeps the
# run offline and deterministic. The extension swallows its own # run offline and deterministic. The extension swallows its own
@@ -43,8 +60,8 @@ main() {
# network note in README.md. Weaker than the Chrome suite's # network note in README.md. Weaker than the Chrome suite's
# fixture interception, and honestly so — it proves no request # fixture interception, and honestly so — it proves no request
# escaped, but it cannot report which ones were attempted. # escaped, but it cannot report which ones were attempted.
# --user: keep files the suite touches owned by the caller, not root. # HOME=/tmp: the image's root home is not a reliable place for the
# HOME=/tmp: the mapped uid has no home directory in the image. # browser profile.
# #
# No --privileged. Firefox's sandbox logs # No --privileged. Firefox's sandbox logs
# "CanCreateUserNamespace() clone() failure: EPERM" on startup here; # "CanCreateUserNamespace() clone() failure: EPERM" on startup here;
@@ -52,11 +69,8 @@ main() {
docker run --rm \ docker run --rm \
--shm-size=1g \ --shm-size=1g \
--network none \ --network none \
--user "$(id -u):$(id -g)" \
-e HOME=/tmp \ -e HOME=/tmp \
-v "$ROOT:/work" \ "$(cat "$IIDFILE")" \
-w /work \
"$IMAGE" \
node tests/e2e/firefox/run.js dist/firefox node tests/e2e/firefox/run.js dist/firefox
} }

View File

@@ -24,6 +24,7 @@ const {
TX_STAGE_VERIFY, TX_STAGE_VERIFY,
TX_STAGE_BROADCAST, TX_STAGE_BROADCAST,
TX_STAGE_INFLIGHT, TX_STAGE_INFLIGHT,
TX_STAGE_NONCE,
} = require("../shared/approvalVerify"); } = require("../shared/approvalVerify");
const { prepareApprovalTx } = require("../shared/approvalTx"); const { prepareApprovalTx } = require("../shared/approvalTx");
const { const {
@@ -57,6 +58,114 @@ const connectedSites = {};
// Pending approval requests: { id: { origin, hostname, resolve } } // Pending approval requests: { id: { origin, hostname, resolve } }
const pendingApprovals = {}; const pendingApprovals = {};
// One transaction approval at a time, wallet-wide.
//
// The transaction a site asks for is populated before its approval window
// opens, so that the object the user is shown is the object the signed
// artifact is verified against. Populating fixes the nonce. Two requests
// populated concurrently therefore take the SAME nonce — the node reports the
// same pending count to both, neither having been broadcast — and whichever is
// broadcast second is refused by the network for a nonce it can never be
// re-signed at, because re-signing it would mean signing something other than
// what was displayed.
//
// So the second request is refused while the first is unanswered. It is
// refused before anything is populated, so no second nonce is allocated at
// all, and while the page is still waiting with nothing on screen. The
// alternatives were considered and rejected in
// https://git.eeqj.de/sneak/AutistMask/issues/271: populating again at Confirm
// puts a nonce on screen that is not the nonce that gets signed, and
// allocating around in-flight approvals makes the wallet's own bookkeeping the
// authority on a nonce the network has not accepted, which an abandoned
// approval then leaves a hole in.
//
// Sign approvals are not gated: a signature consumes no nonce.
//
// The slot is null when free, and otherwise the handle of the request holding
// it. Once that request has raised its approval the handle carries the
// approval's id, so that retiring the approval frees the slot: every exit from
// pendingApprovals goes through settleApproval(), which makes that one hook
// complete. The holder's own finally is the backstop for the interval before
// the approval exists.
let txApprovalSlot = null;
// EIP-1474 "resource unavailable": the standard code for a request that is
// refused because another one is already pending.
const TX_APPROVAL_PENDING_CODE = -32002;
// True at every moment this can be sent: the slot is taken immediately before
// the transaction is populated, so the other request is either being prepared
// or on screen. It does not claim the other one is displayed yet, because for
// the length of one network round trip it is not.
const TX_APPROVAL_PENDING_MESSAGE =
"AutistMask handles one transaction at a time, and another one is" +
" already in progress, so this one was not sent. Please finish that" +
" transaction, then send this one again.";
// Take the slot, or refuse. Nothing awaits between the test and the set, so
// two requests that reach this in the same tick cannot both pass it — the
// position of the call in the handler is irrelevant to that, which is why it
// sits after the authorization checks. A page the wallet is going to refuse
// anyway must not be able to take the slot away from the connected site.
function reserveTxApprovalSlot() {
if (txApprovalSlot) return null;
txApprovalSlot = { approvalId: null };
return txApprovalSlot;
}
// Free the slot, if this handle is still the one holding it.
function releaseTxApprovalSlot(handle) {
if (handle && txApprovalSlot !== handle) return;
txApprovalSlot = null;
}
// Free the slot held on behalf of a retired approval. Called from
// settleApproval() for every approval, and a no-op for the ones the slot was
// not taken for.
function releaseTxApprovalSlotFor(approvalId) {
if (txApprovalSlot && txApprovalSlot.approvalId === approvalId) {
txApprovalSlot = null;
}
}
// Nonces this worker has already handed to the node, per chain and address.
// This is the wallet's own knowledge that a nonce is spent, and it is checked
// before a broadcast rather than after: a node's pending count can lag a
// transaction it has itself just accepted, and a request populated inside that
// window would otherwise be signed and sent at a nonce this wallet has already
// used.
//
// The chain is part of the key because nonce spaces are per chain and the
// wallet switches networks. Without it a nonce spent on one chain would refuse
// that nonce on every other chain — and low nonces overlap across chains as a
// matter of course, so the refusal would be both routine and false.
//
// The record dies with the worker, which is correct rather than merely
// convenient: after a restart the node's count is the only answer available,
// and a transaction of this wallet's that the node has forgotten is one the
// user does want to be able to send again.
const broadcastNonces = {};
function broadcastNoncesFor(chainId, address) {
const key =
String(chainId).toLowerCase() +
":" +
String(address || "").toLowerCase();
if (!broadcastNonces[key]) broadcastNonces[key] = new Set();
return broadcastNonces[key];
}
// An approved transaction's nonce as a decimal string, or null if it cannot be
// read as a number. Verification refuses an unreadable nonce before this is
// ever reached; null here only keeps the record from holding junk.
function approvedNonce(approvedTx) {
try {
return BigInt(approvedTx.nonce).toString();
} catch {
return null;
}
}
async function getState() { async function getState() {
const result = await storageApi.get("autistmask"); const result = await storageApi.get("autistmask");
return ( return (
@@ -148,11 +257,41 @@ function settleApproval(id, result, options) {
const holdsClaim = !!(options && options.holdsClaim); const holdsClaim = !!(options && options.holdsClaim);
if (approval.attemptInFlight && !holdsClaim) return false; if (approval.attemptInFlight && !holdsClaim) return false;
delete pendingApprovals[id]; delete pendingApprovals[id];
// The transaction-approval slot is held for exactly as long as the
// approval it was taken for is alive, and this is the one place an
// approval stops being alive.
releaseTxApprovalSlotFor(id);
approval.resolve(result); approval.resolve(result);
resetPopupUrl(); resetPopupUrl();
return true; return true;
} }
// What a pending approval resolves to when it is given up on rather than
// answered: the window was closed, or could not be opened at all. A tx or sign
// approval answers the requesting page in EIP-1193 shape; a site-connection
// approval answers the connection handler in its own.
function abandonedResult(approval, code, message) {
if (approval.type === "tx" || approval.type === "sign") {
return { error: { code, message } };
}
return { approved: false, remember: false };
}
// A window the user closed without answering is a refusal by the user, which
// is 4001 and the wording every other rejection path already uses.
const APPROVAL_REJECTED_CODE = 4001;
const APPROVAL_REJECTED_MESSAGE = "User rejected the request.";
// The window could not be opened, so the user was never asked. This is the
// wallet failing, not the user refusing, so it does not claim to be a
// rejection: -32603 is the JSON-RPC code for the wallet's own internal
// failure, and the page is told plainly that nothing was shown.
const APPROVAL_WINDOW_FAILED_CODE = -32603;
const APPROVAL_WINDOW_FAILED_MESSAGE =
"AutistMask could not open its approval window, so this request was not" +
" shown to you and nothing was sent.";
// Take exclusive hold of a pending approval for one attempt, or refuse. // Take exclusive hold of a pending approval for one attempt, or refuse.
// //
// An approval that failed retryably has to stay in pendingApprovals, so its // An approval that failed retryably has to stay in pendingApprovals, so its
@@ -172,8 +311,26 @@ function claimApproval(approval) {
// Release an approval whose attempt failed in a way the user can retry. // Release an approval whose attempt failed in a way the user can retry.
// Nothing was broadcast, so the next attempt may claim it. // Nothing was broadcast, so the next attempt may claim it.
//
// Unless the window it would be retried in is already gone. The user closed it
// while the attempt was running and settleApproval() declined then, correctly,
// because the attempt still owned the approval; the attempt has now failed, so
// nothing owns it and nothing can reach it. Left standing it would hold the
// requesting page's promise open forever and, with it, the transaction
// approval slot. It is settled here as the rejection the closed window
// already meant.
function releaseApproval(approval) { function releaseApproval(approval) {
approval.attemptInFlight = false; approval.attemptInFlight = false;
if (approval.windowClosed) {
settleApproval(
approval.id,
abandonedResult(
approval,
APPROVAL_REJECTED_CODE,
APPROVAL_REJECTED_MESSAGE,
),
);
}
} }
// Open approval in a separate popup window. // Open approval in a separate popup window.
@@ -200,10 +357,36 @@ function openApprovalWindow(id) {
); );
} }
windowsApi.create(opts, (win) => { windowsApi.create(opts, (win) => {
const approval = pendingApprovals[id];
if (!approval) {
// Settled while the window was opening — an address switch,
// say. Nothing is waiting on it, and a window showing an
// approval that no longer exists is not left on screen.
if (win) { if (win) {
pendingApprovals[id].windowId = win.id; windowsApi.remove(win.id, () => {
if (runtime.lastError) {
// window already closed
} }
}); });
}
return;
}
if (!win) {
// No window means no way to ever answer this approval, and an
// approval nothing can answer holds the requesting page's
// promise open forever. Settle it now instead.
settleApproval(
id,
abandonedResult(
approval,
APPROVAL_WINDOW_FAILED_CODE,
APPROVAL_WINDOW_FAILED_MESSAGE,
),
);
return;
}
approval.windowId = win.id;
});
}); });
} }
@@ -212,7 +395,7 @@ function openApprovalWindow(id) {
function requestApproval(origin, hostname) { function requestApproval(origin, hostname) {
return new Promise((resolve) => { return new Promise((resolve) => {
const id = crypto.randomUUID(); const id = crypto.randomUUID();
pendingApprovals[id] = { origin, hostname, resolve }; pendingApprovals[id] = { id, origin, hostname, resolve };
if (actionApi && typeof actionApi.openPopup === "function") { if (actionApi && typeof actionApi.openPopup === "function") {
actionApi.setPopup({ actionApi.setPopup({
@@ -243,10 +426,13 @@ function requestApproval(origin, hostname) {
// it is pinned here rather than read again at signing time — an address switch // it is pinned here rather than read again at signing time — an address switch
// between approval and signing must refuse, not sign from an account this // between approval and signing must refuse, not sign from an account this
// screen never named. // screen never named.
function requestTxApproval(origin, hostname, approvedTx, approvedFrom) { // `slot` is the transaction-approval slot its caller holds. Handing the
// approval's id to it is what makes retiring the approval free the slot.
function requestTxApproval(origin, hostname, approvedTx, approvedFrom, slot) {
return new Promise((resolve) => { return new Promise((resolve) => {
const id = crypto.randomUUID(); const id = crypto.randomUUID();
pendingApprovals[id] = { pendingApprovals[id] = {
id,
origin, origin,
hostname, hostname,
approvedTx, approvedTx,
@@ -254,6 +440,7 @@ function requestTxApproval(origin, hostname, approvedTx, approvedFrom) {
resolve, resolve,
type: "tx", type: "tx",
}; };
if (slot) slot.approvalId = id;
openApprovalWindow(id); openApprovalWindow(id);
}); });
@@ -267,6 +454,7 @@ function requestSignApproval(origin, hostname, signParams, approvedFrom) {
return new Promise((resolve) => { return new Promise((resolve) => {
const id = crypto.randomUUID(); const id = crypto.randomUUID();
pendingApprovals[id] = { pendingApprovals[id] = {
id,
origin, origin,
hostname, hostname,
signParams, signParams,
@@ -585,10 +773,30 @@ async function handleRpc(method, params, origin) {
} }
if (method === "eth_sendTransaction") { if (method === "eth_sendTransaction") {
return await handleSendTransaction(params, origin);
}
// Proxy safe read-only methods to the RPC node
if (PROXY_METHODS.includes(method)) {
try {
const result = await proxyRpc(method, params);
return { result };
} catch (e) {
return { error: { message: e.message } };
}
}
return { error: { message: "Unsupported method: " + method } };
}
// The body of eth_sendTransaction, from the connection check through to the
// user's decision. It takes the single transaction-approval slot once it knows
// it is going to populate a transaction, and holds it until the requesting
// page has its answer.
async function handleSendTransaction(params, origin) {
const s = await getState(); const s = await getState();
const activeAddress = await getActiveAddress(); const activeAddress = await getActiveAddress();
if (!activeAddress) if (!activeAddress) return { error: { message: "No accounts available" } };
return { error: { message: "No accounts available" } };
const hostname = extractHostname(origin); const hostname = extractHostname(origin);
const allowed = s.allowedSites[activeAddress] || []; const allowed = s.allowedSites[activeAddress] || [];
@@ -610,6 +818,23 @@ async function handleRpc(method, params, origin) {
}; };
} }
// Everything above refuses without populating anything, so the slot is
// taken here rather than at the top of the handler: a page the wallet was
// never going to serve must not be able to hold the slot and make the
// connected site's own transaction fail as "already in progress". The
// reservation is atomic because nothing awaits between its test and its
// set, not because of where it sits.
const slot = reserveTxApprovalSlot();
if (!slot) {
return {
error: {
code: TX_APPROVAL_PENDING_CODE,
message: TX_APPROVAL_PENDING_MESSAGE,
},
};
}
try {
// Populate here, before any window opens, so that the transaction the // Populate here, before any window opens, so that the transaction the
// user is shown is a complete one and is the same object the signed // user is shown is a complete one and is the same object the signed
// artifact is checked against. A failure raises no approval at all and // artifact is checked against. A failure raises no approval at all and
@@ -644,22 +869,17 @@ async function handleRpc(method, params, origin) {
hostname, hostname,
approvedTx, approvedTx,
activeAddress, activeAddress,
slot,
); );
if (decision.error) return { error: decision.error }; if (decision.error) return { error: decision.error };
return { result: decision.txHash }; return { result: decision.txHash };
} finally {
// Retiring the approval has normally freed the slot already, through
// settleApproval(); this covers the paths that return before an
// approval exists at all, and frees nothing if another request has
// since taken the slot.
releaseTxApprovalSlot(slot);
} }
// Proxy safe read-only methods to the RPC node
if (PROXY_METHODS.includes(method)) {
try {
const result = await proxyRpc(method, params);
return { result };
} catch (e) {
return { error: { message: e.message } };
}
}
return { error: { message: "Unsupported method: " + method } };
} }
// Broadcast chainChanged to all tabs when the network is switched. // Broadcast chainChanged to all tabs when the network is switched.
@@ -694,15 +914,11 @@ async function broadcastAccountsChanged() {
// being signed and broadcast right now, and neither rejecting it to the // being signed and broadcast right now, and neither rejecting it to the
// page nor closing the window it is reporting into is survivable. // page nor closing the window it is reporting into is survivable.
for (const [id, approval] of Object.entries(pendingApprovals)) { for (const [id, approval] of Object.entries(pendingApprovals)) {
const rejection = const rejection = abandonedResult(
approval.type === "tx" || approval.type === "sign" approval,
? { APPROVAL_REJECTED_CODE,
error: { APPROVAL_REJECTED_MESSAGE,
code: 4001, );
message: "User rejected the request.",
},
}
: { approved: false, remember: false };
if (!settleApproval(id, rejection)) continue; if (!settleApproval(id, rejection)) continue;
if (approval.windowId) { if (approval.windowId) {
windowsApi.remove(approval.windowId, () => { windowsApi.remove(approval.windowId, () => {
@@ -831,21 +1047,19 @@ startBackgroundJobs();
// verify and broadcast it is waiting on, so a user closing an apparently-hung // 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. // window is an ordinary event with an attempt already in flight behind it.
// settleApproval() refuses those, which leaves the attempt to report its real // settleApproval() refuses those, which leaves the attempt to report its real
// outcome to the page. // outcome to the page — and the window is recorded as gone, so that an attempt
// which then fails retryably settles instead of waiting in a window that no
// longer exists.
if (windowsApi && windowsApi.onRemoved) { if (windowsApi && windowsApi.onRemoved) {
windowsApi.onRemoved.addListener((windowId) => { windowsApi.onRemoved.addListener((windowId) => {
for (const [id, approval] of Object.entries(pendingApprovals)) { for (const [id, approval] of Object.entries(pendingApprovals)) {
if (approval.windowId !== windowId) continue; if (approval.windowId !== windowId) continue;
const rejection = const rejection = abandonedResult(
approval.type === "tx" || approval.type === "sign" approval,
? { APPROVAL_REJECTED_CODE,
error: { APPROVAL_REJECTED_MESSAGE,
code: 4001, );
message: "User rejected the request.", if (!settleApproval(id, rejection)) approval.windowClosed = true;
},
}
: { approved: false, remember: false };
settleApproval(id, rejection);
} }
}); });
} }
@@ -959,7 +1173,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
sendResponse({ sendResponse({
error: outcome.error, error: outcome.error,
retryable: outcome.retryable, retryable: outcome.retryable,
stage: TX_STAGE_SIGN, stage: outcome.stage,
}); });
return false; return false;
} }
@@ -975,8 +1189,15 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
} }
(async () => { (async () => {
// The chain this attempt is on, read once. Verification below
// refuses an artifact signed for any other chain, and the nonce
// record is both consulted and written under this one, so a
// network switch part-way through cannot make the check and the
// record disagree about which chain the nonce was spent on.
let chainId;
try { try {
await loadState(); await loadState();
chainId = currentNetwork().chainId;
const activeAddress = await getActiveAddress(); const activeAddress = await getActiveAddress();
// An address switch between approval and signing refuses. The // An address switch between approval and signing refuses. The
// approval named one account; signing from whichever account // approval named one account; signing from whichever account
@@ -999,7 +1220,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
msg.rawSignedTx, msg.rawSignedTx,
approval.approvedTx, approval.approvedTx,
approval.approvedFrom, approval.approvedFrom,
currentNetwork().chainId, chainId,
); );
} catch (e) { } catch (e) {
// A signed transaction that is not the approved one is not // A signed transaction that is not the approved one is not
@@ -1019,7 +1240,31 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
sendResponse({ sendResponse({
error: outcome.error, error: outcome.error,
retryable: outcome.retryable, retryable: outcome.retryable,
stage: TX_STAGE_VERIFY, stage: outcome.stage,
});
return;
}
// A nonce this worker has already broadcast for this address on
// this chain. The node is not asked: it has answered once already,
// and the wallet holding the receipt of that answer is what makes
// this failure one the user can be told did not reach the network.
// A nonce spent on another chain is not spent here — the chains
// count separately, and refusing across them would block ordinary
// use with a message that is not true.
const nonce = approvedNonce(approval.approvedTx);
const spent = broadcastNoncesFor(chainId, approval.approvedFrom);
if (nonce !== null && spent.has(nonce)) {
const outcome = describeTxFailure(TX_STAGE_NONCE, null);
settleApproval(
msg.id,
{ error: { message: outcome.error } },
{ holdsClaim: true },
);
sendResponse({
error: outcome.error,
retryable: outcome.retryable,
stage: outcome.stage,
}); });
return; return;
} }
@@ -1027,6 +1272,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
try { try {
const provider = getProvider(state.rpcUrl); const provider = getProvider(state.rpcUrl);
const tx = await provider.broadcastTransaction(msg.rawSignedTx); const tx = await provider.broadcastTransaction(msg.rawSignedTx);
if (nonce !== null) spent.add(nonce);
settleApproval( settleApproval(
msg.id, msg.id,
{ txHash: tx.hash }, { txHash: tx.hash },
@@ -1039,6 +1285,11 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
// tell a transaction that never left from one already in the // tell a transaction that never left from one already in the
// mempool. The page has been given its outcome for this // mempool. The page has been given its outcome for this
// request; a second attempt would report a second one. // request; a second attempt would report a second one.
//
// Unless the node blamed the nonce, which is the one answer
// that says plainly it did not take the transaction:
// describeTxFailure() reclassifies that, and the stage it
// returns is the one reported.
const outcome = describeTxFailure(TX_STAGE_BROADCAST, e); const outcome = describeTxFailure(TX_STAGE_BROADCAST, e);
settleApproval( settleApproval(
msg.id, msg.id,
@@ -1048,7 +1299,7 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
sendResponse({ sendResponse({
error: outcome.error, error: outcome.error,
retryable: outcome.retryable, retryable: outcome.retryable,
stage: TX_STAGE_BROADCAST, stage: outcome.stage,
}); });
} }
})(); })();

View File

@@ -13,7 +13,7 @@ const {
pushCurrentView, pushCurrentView,
} = require("./helpers"); } = require("./helpers");
const { state, currentAddress, saveState } = require("../../shared/state"); const { state, currentAddress, saveState } = require("../../shared/state");
const { formatUsd, getAddressValueUsd } = require("../../shared/prices"); const { formatAddressTotal, getAddressValue } = require("../../shared/prices");
const { const {
fetchRecentTransactions, fetchRecentTransactions,
filterTransactions, filterTransactions,
@@ -64,7 +64,7 @@ function show() {
}); });
$("address-line").dataset.full = addr.address; $("address-line").dataset.full = addr.address;
attachCopyHandlers($("address-line")); attachCopyHandlers($("address-line"));
const usdTotal = formatUsd(getAddressValueUsd(addr)); const usdTotal = formatAddressTotal(getAddressValue(addr));
$("address-usd-total").innerHTML = usdTotal || "&nbsp;"; $("address-usd-total").innerHTML = usdTotal || "&nbsp;";
const ensEl = $("address-ens"); const ensEl = $("address-ens");
// ENS is now shown inside renderAddressHtml, hide the separate element // ENS is now shown inside renderAddressHtml, hide the separate element

View File

@@ -18,11 +18,7 @@ const {
} = require("./helpers"); } = require("./helpers");
const { state, currentAddress, saveState } = require("../../shared/state"); const { state, currentAddress, saveState } = require("../../shared/state");
const { TOKEN_BY_ADDRESS, resolveSymbol } = require("../../shared/tokenList"); const { TOKEN_BY_ADDRESS, resolveSymbol } = require("../../shared/tokenList");
const { const { formatUsd, getPrice } = require("../../shared/prices");
formatUsd,
getPrice,
getAddressValueUsd,
} = require("../../shared/prices");
const { const {
fetchRecentTransactions, fetchRecentTransactions,
filterTransactions, filterTransactions,

View File

@@ -17,7 +17,7 @@ const {
addressHoldsFunds, addressHoldsFunds,
balanceLinesForAddress, balanceLinesForAddress,
} = require("./helpers"); } = require("./helpers");
const { formatUsd, getAddressValueUsd } = require("../../shared/prices"); const { formatAddressTotal, getAddressValue } = require("../../shared/prices");
const { walletHasRecoveryPhrase } = require("../../shared/wallet"); const { walletHasRecoveryPhrase } = require("../../shared/wallet");
const { state, saveState } = require("../../shared/state"); const { state, saveState } = require("../../shared/state");
const { const {
@@ -84,16 +84,16 @@ function recoveryPathText(wallet) {
// own: the rendered lines round to four decimals, so a sentence built from a // 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. // 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 // 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 // AddressDetail, followed by the USD total when there is one to give — no
// testnet and before the first price fetch, where the line is left off rather // total line at all on testnet or before the first price fetch, and no figure
// than printed as $0.00). // when every holding here is one with no price, since "$0.00" directly under
// "This address holds a balance." is a contradiction.
function balanceWarningHtml(addr) { function balanceWarningHtml(addr) {
if (!addressHoldsFunds(addr)) return "&nbsp;"; if (!addressHoldsFunds(addr)) return "&nbsp;";
const usd = getAddressValueUsd(addr); const line = formatAddressTotal(getAddressValue(addr));
const total = const total = line
usd === null ? `<div class="text-xs text-muted mt-1">${line}</div>`
? "" : "";
: `<div class="text-xs text-muted mt-1">Total: ${formatUsd(usd)}</div>`;
return ( return (
`<p class="mb-1">This address holds a balance. Removing it does not ` + `<p class="mb-1">This address holds a balance. Removing it does not ` +
`move or spend anything; the balance stays at the address.</p>` + `move or spend anything; the balance stays at the address.</p>` +

View File

@@ -1,11 +1,7 @@
// Shared DOM helpers used by all views. // Shared DOM helpers used by all views.
const { isDebug } = require("../../shared/log"); const { isDebug } = require("../../shared/log");
const { const { formatUsd, getPrice } = require("../../shared/prices");
formatUsd,
getPrice,
getAddressValueUsd,
} = require("../../shared/prices");
const { state, saveState, currentNetwork } = require("../../shared/state"); const { state, saveState, currentNetwork } = require("../../shared/state");
const { markViewRendered } = require("../viewRouter"); const { markViewRendered } = require("../viewRouter");

View File

@@ -28,8 +28,9 @@ const {
} = require("../../shared/walletDefects"); } = require("../../shared/walletDefects");
const { const {
formatUsd, formatUsd,
formatAddressTotal,
getPrice, getPrice,
getAddressValueUsd, getAddressValue,
} = require("../../shared/prices"); } = require("../../shared/prices");
const { const {
fetchRecentTransactions, fetchRecentTransactions,
@@ -71,9 +72,7 @@ function renderTotalValue() {
el.textContent = ethStr + ethUsd; el.textContent = ethStr + ethUsd;
if (subEl) { if (subEl) {
const totalUsd = getAddressValueUsd(addr); subEl.innerHTML = formatAddressTotal(getAddressValue(addr)) || "&nbsp;";
subEl.innerHTML =
totalUsd !== null ? "Total: " + formatUsd(totalUsd) : "&nbsp;";
} }
} }
@@ -257,8 +256,8 @@ function walletListHtml() {
html += `<span class="flex items-center break-all">${addr.ensName ? "" : dot}${addr.address}</span>`; html += `<span class="flex items-center break-all">${addr.ensName ? "" : dot}${addr.address}</span>`;
html += `<span class="flex-shrink-0 ml-1">${infoBtn}${removeBtn}</span>`; html += `<span class="flex-shrink-0 ml-1">${infoBtn}${removeBtn}</span>`;
html += `</div>`; html += `</div>`;
const addrUsd = formatUsd(getAddressValueUsd(addr)); const addrTotal = formatAddressTotal(getAddressValue(addr));
html += `<div class="text-xs text-muted text-right min-h-[1rem]">${addrUsd || "&nbsp;"}</div>`; html += `<div class="text-xs text-muted text-right min-h-[1rem]">${addrTotal || "&nbsp;"}</div>`;
html += balanceLinesForAddress( html += balanceLinesForAddress(
addr, addr,
state.trackedTokens, state.trackedTokens,

View File

@@ -169,10 +169,7 @@ function renderWalletListSettings() {
function show() { function show() {
$("settings-rpc").value = state.rpcUrl; $("settings-rpc").value = state.rpcUrl;
$("settings-blockscout").value = state.blockscoutUrl; $("settings-blockscout").value = state.blockscoutUrl;
const networkSelect = $("settings-network"); $("settings-network").value = state.networkId;
if (networkSelect) {
networkSelect.value = state.networkId;
}
renderTrackedTokens(); renderTrackedTokens();
renderSiteLists(); renderSiteLists();
renderWalletListSettings(); renderWalletListSettings();
@@ -284,7 +281,6 @@ function init(ctx) {
}); });
const networkSelect = $("settings-network"); const networkSelect = $("settings-network");
if (networkSelect) {
networkSelect.addEventListener("change", async () => { networkSelect.addEventListener("change", async () => {
const newId = networkSelect.value; const newId = networkSelect.value;
const net = await onChainSwitch(newId); const net = await onChainSwitch(newId);
@@ -292,7 +288,6 @@ function init(ctx) {
$("settings-blockscout").value = state.blockscoutUrl; $("settings-blockscout").value = state.blockscoutUrl;
showFlash("Switched to " + net.name + "."); showFlash("Switched to " + net.name + ".");
}); });
}
$("settings-show-zero-balances").checked = state.showZeroBalanceTokens; $("settings-show-zero-balances").checked = state.showZeroBalanceTokens;
$("settings-show-zero-balances").addEventListener("change", async () => { $("settings-show-zero-balances").addEventListener("change", async () => {

View File

@@ -602,6 +602,12 @@ const TX_STAGE_BROADCAST = "broadcast";
// may yet succeed, so the one thing the popup must not say is "start again // may yet succeed, so the one thing the popup must not say is "start again
// from the site". // from the site".
const TX_STAGE_INFLIGHT = "inflight"; const TX_STAGE_INFLIGHT = "inflight";
// A transaction refused for a nonce that is already spoken for, either by the
// node's own answer or by this wallet's record of what it has broadcast. It is
// the one broadcast-stage failure that is not ambiguous: the transaction was
// not taken, so the user is told it did not reach the network and to send it
// again, rather than being warned that it might already be out there.
const TX_STAGE_NONCE = "nonce";
function errorText(err) { function errorText(err) {
if (typeof err === "string" && err !== "") return err; if (typeof err === "string" && err !== "") return err;
@@ -611,6 +617,59 @@ function errorText(err) {
return "The transaction could not be sent."; return "The transaction could not be sent.";
} }
// Every string a failure might carry its reason in. ethers reports the node's
// own words in `shortMessage`, but a JSON-RPC error it could not classify is
// nested under `error` or `info.error` with the node's message intact, and the
// classification below has to see that too.
function failureTexts(err) {
if (typeof err === "string") return [err];
if (!err || typeof err !== "object") return [];
const texts = [];
for (const text of [err.shortMessage, err.message, err.reason]) {
if (text) texts.push(String(text));
}
const nested = err.error || (err.info && err.info.error);
if (nested && nested.message) texts.push(String(nested.message));
return texts;
}
// What the Ethereum clients say when a transaction's nonce is already spoken
// for: either it is below the account's next nonce, or another transaction is
// sitting in the pool at that nonce and this one did not outbid it. Either way
// the node answered, and its answer was that it did not take this transaction.
//
// "already known" is deliberately absent. A node that says it knows the
// transaction has it, so that transaction did reach the network and the
// ambiguous broadcast wording is the correct one for it.
const NONCE_COLLISION_PATTERNS = [
/nonce too low/i,
/nonce has already been used/i,
/invalid nonce/i,
/oldnonce/i,
/replacement transaction underpriced/i,
/replacement fee too low/i,
];
// ethers' own classification of the same two conditions.
const NONCE_COLLISION_CODES = ["NONCE_EXPIRED", "REPLACEMENT_UNDERPRICED"];
// Whether a failed send is a nonce collision.
function isNonceCollision(err) {
if (!err) return false;
if (err.code && NONCE_COLLISION_CODES.includes(err.code)) return true;
return failureTexts(err).some((text) =>
NONCE_COLLISION_PATTERNS.some((pattern) => pattern.test(text)),
);
}
// What both the requesting page and the popup are told about a nonce
// collision. The node's own words ("nonce too low") are a fragment and are
// replaced rather than passed through: they are not a sentence, and they say
// less than the wallet knows.
const NONCE_COLLISION_MESSAGE =
"The transaction was not sent, because its nonce had already been used" +
" by another transaction.";
// What the background does with a pending transaction approval after a failed // What the background does with a pending transaction approval after a failed
// attempt: what it tells the popup, and whether the approval is spent // 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 // (resolved to the requesting page as an error and deleted) or left standing
@@ -627,12 +686,31 @@ function errorText(err) {
// that never left from one that is already in the mempool. The approval is // that never left from one that is already in the mempool. The approval is
// spent and the requesting page has been given its outcome; a second // spent and the requesting page has been given its outcome; a second
// attempt against it would report a second outcome for one request. // attempt against it would report a second outcome for one request.
// - nonce: terminal too, and the one case where the wallet does know the
// transaction never left. The approval carries a nonce that is spent, so
// the artifact signed against it can never be accepted and the user is told
// to send it again from the site.
//
// The stage comes back out because a broadcast failure the node blamed on the
// nonce is reclassified here; the caller reports the stage this returns rather
// than the one it passed in.
function describeTxFailure(stage, err) { function describeTxFailure(stage, err) {
if (
stage === TX_STAGE_NONCE ||
(stage === TX_STAGE_BROADCAST && isNonceCollision(err))
) {
return {
error: NONCE_COLLISION_MESSAGE,
retryable: false,
spendApproval: true,
stage: TX_STAGE_NONCE,
};
}
const error = errorText(err); const error = errorText(err);
const retryable = const retryable =
stage === TX_STAGE_SIGN || stage === TX_STAGE_SIGN ||
(stage === TX_STAGE_VERIFY && failureIsRetryable(err)); (stage === TX_STAGE_VERIFY && failureIsRetryable(err));
return { error, retryable, spendApproval: !retryable }; return { error, retryable, spendApproval: !retryable, stage };
} }
// What the popup shows and does after the background reports a failed signing // What the popup shows and does after the background reports a failed signing
@@ -642,14 +720,20 @@ function describeTxFailure(stage, err) {
// //
// A failed broadcast gets its own wording: the transaction may already be on // 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 // the network, so telling the user to start again from the site is exactly the
// wrong instruction. // wrong instruction. A nonce collision is the exception to that exception —
// the transaction demonstrably did not go out, and saying it might have would
// send the user hunting for a transaction that does not exist.
function describeSigningFailure(response, fallbackMessage) { function describeSigningFailure(response, fallbackMessage) {
let message = (response && response.error) || fallbackMessage; let message = (response && response.error) || fallbackMessage;
if (!/[.!?]$/.test(message)) message += "."; if (!/[.!?]$/.test(message)) message += ".";
const retryable = !!(response && response.retryable); const retryable = !!(response && response.retryable);
const stage = response && response.stage; const stage = response && response.stage;
if (!retryable) { if (!retryable) {
if (stage === TX_STAGE_BROADCAST) { if (stage === TX_STAGE_NONCE) {
message +=
" The transaction did not reach the network." +
" Please send it again from the site.";
} else if (stage === TX_STAGE_BROADCAST) {
message += message +=
" The transaction may still have reached the network." + " The transaction may still have reached the network." +
" Check the account before sending it again."; " Check the account before sending it again.";
@@ -675,9 +759,11 @@ module.exports = {
assertWithinCeilings, assertWithinCeilings,
sameAddress, sameAddress,
failureIsRetryable, failureIsRetryable,
isNonceCollision,
describeTxFailure, describeTxFailure,
describeSigningFailure, describeSigningFailure,
ApprovalMismatchError, ApprovalMismatchError,
NONCE_COLLISION_MESSAGE,
ALLOWED_TX_TYPES, ALLOWED_TX_TYPES,
SERIALIZED_FIELDS, SERIALIZED_FIELDS,
FORBIDDEN_FIELDS, FORBIDDEN_FIELDS,
@@ -686,6 +772,7 @@ module.exports = {
TX_STAGE_VERIFY, TX_STAGE_VERIFY,
TX_STAGE_BROADCAST, TX_STAGE_BROADCAST,
TX_STAGE_INFLIGHT, TX_STAGE_INFLIGHT,
TX_STAGE_NONCE,
MAX_GAS_LIMIT, MAX_GAS_LIMIT,
MAX_FEE_PER_GAS, MAX_FEE_PER_GAS,
}; };

View File

@@ -55,42 +55,77 @@ function formatUsd(amount) {
); );
} }
function getAddressValueUsd(addr) { // What an address is worth, as { usd, partial }.
//
// Prices are fetched for the top 25 tokens only, so an address can hold real
// assets this code has no price for. Adding up the priced ones and calling the
// result the total states a number the holdings do not support: an address
// holding nothing but unpriced tokens comes out at $0.00, which tells the user
// their address is worth nothing when it may hold a great deal. Worth zero and
// worth an unknown amount are separate facts and get separate fields, the same
// way an absent holders_count is not a count of zero.
//
// usd: the value of the holdings a price is known for, or null when
// nothing is knowable at all — testnet, or before the first fetch.
// partial: the address also holds a token with no price, so usd is a floor
// and not the total.
//
// Render it through formatAddressTotal() rather than reading usd alone.
function getAddressValue(addr) {
const { currentNetwork } = require("./state"); const { currentNetwork } = require("./state");
if (currentNetwork().isTestnet) return null; if (currentNetwork().isTestnet) return { usd: null, partial: false };
if (!prices.ETH) return null; if (!prices.ETH) return { usd: null, partial: false };
let total = 0; let usd = parseFloat(addr.balance || "0") * prices.ETH;
const ethBal = parseFloat(addr.balance || "0"); let partial = false;
total += ethBal * prices.ETH;
for (const token of addr.tokenBalances || []) { for (const token of addr.tokenBalances || []) {
const tokenBal = parseFloat(token.balance || "0"); const tokenBal = parseFloat(token.balance || "0");
if (tokenBal > 0 && prices[token.symbol]) { // A balance of zero is not a holding: it can neither add to the total
total += tokenBal * prices[token.symbol]; // nor make it incomplete.
if (!(tokenBal > 0)) continue;
if (prices[token.symbol]) {
usd += tokenBal * prices[token.symbol];
} else {
partial = true;
} }
} }
return total; return { usd, partial };
} }
function getWalletValueUsd(wallet) { // The same pair for a whole wallet, and for every wallet at once. One
const { currentNetwork } = require("./state"); // unpriced holding anywhere makes the sum a floor, so partial carries up.
if (currentNetwork().isTestnet) return null; function getWalletValue(wallet) {
if (!prices.ETH) return null; return sumValues(wallet.addresses.map(getAddressValue));
let total = 0;
for (const addr of wallet.addresses) {
total += getAddressValueUsd(addr);
}
return total;
} }
function getTotalValueUsd(wallets) { function getTotalValue(wallets) {
const { currentNetwork } = require("./state"); return sumValues(wallets.map(getWalletValue));
if (currentNetwork().isTestnet) return null; }
if (!prices.ETH) return null;
let total = 0; function sumValues(values) {
for (const wallet of wallets) { let usd = null;
total += getWalletValueUsd(wallet); let partial = false;
for (const value of values) {
if (value.usd === null) continue;
usd = (usd === null ? 0 : usd) + value.usd;
partial = partial || value.partial;
} }
return total; return { usd, partial };
}
// The one rendering of an address total, so no screen says it differently.
//
// A partial total is shown and named as partial: the figure is the ETH and
// priced tokens the user does hold, which is worth having, and suppressing it
// would throw away a number that is correct as far as it goes. What is never
// shown is a figure covering no holdings at all — the $0.00 sum of an empty
// set beside a list of tokens is the bug this replaces.
function formatAddressTotal(value) {
if (!value || value.usd === null) return "";
if (!value.partial) return "Total: " + formatUsd(value.usd);
if (value.usd > 0) {
return "Total: " + formatUsd(value.usd) + " plus unpriced tokens";
}
return "Total: unpriced tokens only";
} }
module.exports = { module.exports = {
@@ -99,7 +134,8 @@ module.exports = {
clearPrices, clearPrices,
getPrice, getPrice,
formatUsd, formatUsd,
getAddressValueUsd, formatAddressTotal,
getWalletValueUsd, getAddressValue,
getTotalValueUsd, getWalletValue,
getTotalValue,
}; };

238
tests/addressValue.test.js Normal file
View File

@@ -0,0 +1,238 @@
// The USD total of an address that holds something this build cannot price
// (issue #261).
//
// Prices exist for the top 25 tokens only, so an address can hold real assets
// with no price attached. Summing what is priced and printing the result as
// the total says "$0.00" for an address holding nothing but unpriced tokens —
// worth-nothing and worth-an-unknown-amount collapsed into one number, in the
// direction that matters. The two are separate facts here, the same way an
// absent holders_count is not a count of zero.
//
// The value and its rendering are asserted directly, and then through the two
// call sites that return their markup as a string: the wallet list on Home and
// the balance warning on the address-removal confirmation. AddressDetail and
// the Home summary line render into the DOM and are covered by tests/e2e.
// helpers.js pulls in state.js, which reads chrome.storage.local at load.
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
const {
prices,
clearPrices,
getAddressValue,
getWalletValue,
getTotalValue,
formatAddressTotal,
} = require("../src/shared/prices");
const { state } = require("../src/shared/state");
const { walletListHtml } = require("../src/popup/views/home");
const { balanceWarningHtml } = require("../src/popup/views/deleteAddress");
const USDC = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
const NOVEL = "0x1111111111111111111111111111111111111111";
// No ETH, and a token no price is known for. The case the user is told is
// worth $0.00 today.
const UNPRICED_ONLY = {
address: "0x" + "a".repeat(40),
balance: "0",
tokenBalances: [{ address: NOVEL, symbol: "NOVEL", balance: "5000.0" }],
};
// Nothing at all: the address really is worth zero.
const EMPTY = {
address: "0x" + "b".repeat(40),
balance: "0",
tokenBalances: [],
};
// Every holding priced.
const FULLY_PRICED = {
address: "0x" + "c".repeat(40),
balance: "1.5",
tokenBalances: [{ address: USDC, symbol: "USDC", balance: "2500.0" }],
};
// Part priced, part not: 1.5 ETH plus a token with no price.
const PARTLY_PRICED = {
address: "0x" + "d".repeat(40),
balance: "1.5",
tokenBalances: [{ address: NOVEL, symbol: "NOVEL", balance: "5000.0" }],
};
beforeEach(() => {
clearPrices();
prices.ETH = 2000;
prices.USDC = 1;
state.wallets = [];
state.trackedTokens = [];
state.showZeroBalanceTokens = false;
state.activeAddress = null;
});
afterEach(() => {
clearPrices();
});
// The total line only, in each of the two markup-returning call sites. The
// ETH balance line above it legitimately reads $0.00 for an address with no
// ETH, so the assertions have to name the line under test.
function walletListTotal(addr) {
state.wallets = [{ name: "Wallet 1", type: "hd", addresses: [addr] }];
const match = walletListHtml().match(/min-h-\[1rem\]">([^<]*)</);
return match && match[1];
}
function removalWarningTotal(addr) {
const match = balanceWarningHtml(addr).match(/mt-1">([^<]*)</);
return match && match[1];
}
describe("the value of an address, and whether it is the whole value", () => {
test("an address holding only unpriced tokens has an incomplete value", () => {
expect(getAddressValue(UNPRICED_ONLY)).toEqual({
usd: 0,
partial: true,
});
});
test("an address holding nothing is complete, and zero", () => {
expect(getAddressValue(EMPTY)).toEqual({ usd: 0, partial: false });
});
test("a fully priced address is complete, and unchanged", () => {
expect(getAddressValue(FULLY_PRICED)).toEqual({
usd: 5500,
partial: false,
});
});
test("a partly priced address keeps the part it can price", () => {
expect(getAddressValue(PARTLY_PRICED)).toEqual({
usd: 3000,
partial: true,
});
});
// A token balance of zero is not a holding, so it cannot make the total
// incomplete: an address with a spent-out unpriced token is worth zero.
test("a zero balance in an unpriced token leaves the value complete", () => {
const addr = {
address: "0x1",
balance: "0",
tokenBalances: [{ address: NOVEL, symbol: "NOVEL", balance: "0" }],
};
expect(getAddressValue(addr)).toEqual({ usd: 0, partial: false });
});
// Before the first price fetch, and on testnet, nothing is knowable: that
// is a third state, and it stays distinct from both of the others.
test("no prices at all means no value, not an incomplete one", () => {
clearPrices();
expect(getAddressValue(FULLY_PRICED)).toEqual({
usd: null,
partial: false,
});
});
test("one unpriced holding makes a wallet and the grand total partial", () => {
const wallet = { addresses: [FULLY_PRICED, UNPRICED_ONLY] };
expect(getWalletValue(wallet)).toEqual({ usd: 5500, partial: true });
expect(getTotalValue([wallet])).toEqual({ usd: 5500, partial: true });
});
test("a wallet of fully priced addresses stays complete", () => {
const wallet = { addresses: [FULLY_PRICED, EMPTY] };
expect(getWalletValue(wallet)).toEqual({ usd: 5500, partial: false });
});
});
describe("how that value is written on screen", () => {
test("a complete total is the figure", () => {
expect(formatAddressTotal(getAddressValue(FULLY_PRICED))).toBe(
"Total: $5,500.00",
);
});
test("an address worth zero says so", () => {
expect(formatAddressTotal(getAddressValue(EMPTY))).toBe("Total: $0.00");
});
// The figure is still worth having — it is the ETH the user does hold —
// but on its own it understates the address, so it is named as partial.
test("a partly priced total is given, and marked as partial", () => {
expect(formatAddressTotal(getAddressValue(PARTLY_PRICED))).toBe(
"Total: $3,000.00 plus unpriced tokens",
);
});
// Nothing priced is held, so there is no figure to give: printing the
// $0.00 sum of an empty set is the bug.
test("a total with nothing priced in it gives no figure", () => {
const line = formatAddressTotal(getAddressValue(UNPRICED_ONLY));
expect(line).toBe("Total: unpriced tokens only");
expect(line).not.toContain("$");
});
test("an unknown value is written as nothing at all", () => {
clearPrices();
expect(formatAddressTotal(getAddressValue(FULLY_PRICED))).toBe("");
});
});
describe("the wallet list on Home", () => {
test("an address holding only unpriced tokens is not totalled at $0.00", () => {
expect(walletListTotal(UNPRICED_ONLY)).toBe(
"Total: unpriced tokens only",
);
});
test("an address holding nothing is still totalled at $0.00", () => {
expect(walletListTotal(EMPTY)).toBe("Total: $0.00");
});
test("a fully priced address shows its total", () => {
expect(walletListTotal(FULLY_PRICED)).toBe("Total: $5,500.00");
});
test("a partly priced address shows the priced part, marked partial", () => {
expect(walletListTotal(PARTLY_PRICED)).toBe(
"Total: $3,000.00 plus unpriced tokens",
);
});
test("an address whose value is unknown keeps its blank line", () => {
clearPrices();
expect(walletListTotal(FULLY_PRICED)).toBe("&nbsp;");
});
});
describe("the balance warning on the address-removal confirmation", () => {
// "This address holds a balance." followed by "Total: $0.00" is a flat
// contradiction, on the one screen whose job is to warn.
test("an address holding only unpriced tokens is not totalled at $0.00", () => {
expect(balanceWarningHtml(UNPRICED_ONLY)).toContain(
"This address holds a balance.",
);
expect(removalWarningTotal(UNPRICED_ONLY)).toBe(
"Total: unpriced tokens only",
);
});
test("a fully priced address still shows its total", () => {
expect(removalWarningTotal(FULLY_PRICED)).toBe("Total: $5,500.00");
});
test("a partly priced address shows the priced part, marked partial", () => {
expect(removalWarningTotal(PARTLY_PRICED)).toBe(
"Total: $3,000.00 plus unpriced tokens",
);
});
test("no total line is written when the value is unknown", () => {
clearPrices();
expect(removalWarningTotal(FULLY_PRICED)).toBe(null);
});
});

View File

@@ -14,8 +14,10 @@ const {
assertWithinCeilings, assertWithinCeilings,
sameAddress, sameAddress,
failureIsRetryable, failureIsRetryable,
isNonceCollision,
describeTxFailure, describeTxFailure,
describeSigningFailure, describeSigningFailure,
NONCE_COLLISION_MESSAGE,
ALLOWED_TX_TYPES, ALLOWED_TX_TYPES,
SERIALIZED_FIELDS, SERIALIZED_FIELDS,
FORBIDDEN_FIELDS, FORBIDDEN_FIELDS,
@@ -23,6 +25,7 @@ const {
TX_STAGE_SIGN, TX_STAGE_SIGN,
TX_STAGE_VERIFY, TX_STAGE_VERIFY,
TX_STAGE_BROADCAST, TX_STAGE_BROADCAST,
TX_STAGE_NONCE,
MAX_GAS_LIMIT, MAX_GAS_LIMIT,
MAX_FEE_PER_GAS, MAX_FEE_PER_GAS,
} = require("../src/shared/approvalVerify"); } = require("../src/shared/approvalVerify");
@@ -1191,7 +1194,6 @@ describe("signing failure and retry", () => {
"already known", "already known",
"timeout of 30000ms exceeded", "timeout of 30000ms exceeded",
"could not coalesce error", "could not coalesce error",
"replacement transaction underpriced",
]) { ]) {
const outcome = describeTxFailure( const outcome = describeTxFailure(
TX_STAGE_BROADCAST, TX_STAGE_BROADCAST,
@@ -1199,10 +1201,76 @@ describe("signing failure and retry", () => {
); );
expect(outcome.retryable).toBe(false); expect(outcome.retryable).toBe(false);
expect(outcome.spendApproval).toBe(true); expect(outcome.spendApproval).toBe(true);
expect(outcome.stage).toBe(TX_STAGE_BROADCAST);
expect(outcome.error).toBe(message); expect(outcome.error).toBe(message);
} }
}); });
// The one broadcast failure that is not ambiguous. The node answered, and
// its answer was that the nonce was already spoken for, so this
// transaction is not in a mempool anywhere.
test("a nonce the node refused is classified however it was worded", () => {
for (const err of [
new Error("nonce too low"),
new Error("replacement transaction underpriced"),
Object.assign(new Error("could not coalesce error"), {
code: "NONCE_EXPIRED",
}),
Object.assign(new Error("could not coalesce error"), {
code: "REPLACEMENT_UNDERPRICED",
}),
// The shape ethers hands up when it could not classify the node's
// error itself: the node's own words are nested underneath.
Object.assign(new Error("could not coalesce error"), {
info: { error: { code: -32000, message: "OldNonce" } },
}),
]) {
const outcome = describeTxFailure(TX_STAGE_BROADCAST, err);
expect(
describeSigningFailure(
outcome,
"The transaction could not be sent.",
).message,
).toMatch(/did not reach the network/);
expect(outcome.retryable).toBe(false);
expect(outcome.spendApproval).toBe(true);
expect(outcome.error).toBe(NONCE_COLLISION_MESSAGE);
expect(outcome.stage).toBe(TX_STAGE_NONCE);
expect(isNonceCollision(err)).toBe(true);
}
});
// A node that says it knows the transaction has it, so it did reach the
// network and the ambiguous wording is the correct one.
test("already known is not a nonce collision", () => {
const err = new Error("already known");
const outcome = describeTxFailure(TX_STAGE_BROADCAST, err);
expect(
describeSigningFailure(
outcome,
"The transaction could not be sent.",
).message,
).toMatch(/may still have reached the network/);
expect(outcome.stage).toBe(TX_STAGE_BROADCAST);
expect(isNonceCollision(err)).toBe(false);
});
test("a nonce collision says the transaction did not reach the network", () => {
const outcome = describeTxFailure(
TX_STAGE_BROADCAST,
new Error("nonce too low"),
);
const copy = describeSigningFailure(
outcome,
"The transaction could not be sent.",
);
expect(copy.retryable).toBe(false);
expect(copy.message).toMatch(/did not reach the network/);
expect(copy.message).not.toMatch(/may still have reached the network/);
expect(copy.message).toMatch(/Please send it again from the site\.$/);
expect(copy.message).toMatch(/^[A-Z].*\.$/);
});
test("a failed broadcast does not tell the user to send it again", () => { test("a failed broadcast does not tell the user to send it again", () => {
const outcome = describeSigningFailure( const outcome = describeSigningFailure(
{ {

View File

@@ -30,6 +30,8 @@ const RECIPIENT = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const ORIGIN = "https://dapp.example"; const ORIGIN = "https://dapp.example";
const HOSTNAME = "dapp.example"; const HOSTNAME = "dapp.example";
// A page the wallet has never been connected to, whose requests are refused.
const UNCONNECTED_ORIGIN = "https://stranger.example";
const EXT_URL = "chrome-extension://autistmask/"; const EXT_URL = "chrome-extension://autistmask/";
// What the dApp asks for: no nonce, no gas, no fees. This is the shape that // What the dApp asks for: no nonce, no gas, no fees. This is the shape that
@@ -51,10 +53,16 @@ const MESSAGE = "0x48656c6c6f204175746973744d61736b";
// The transaction the background populates and the approval screen displays. // The transaction the background populates and the approval screen displays.
// The nonce is a parameter because the duplicate case turns on two artifacts // The nonce is a parameter because the duplicate case turns on two artifacts
// differing in a field the dApp fixed nothing for. // differing in a field the dApp fixed nothing for.
function populated(nonce) { // The two chains the tests switch between, as both forms the code uses: the
// hex chain id the wallet's network record carries, and the number the node
// and the signed artifact carry.
const MAINNET = { hex: "0x1", num: 1 };
const SEPOLIA = { hex: "0xaa36a7", num: 11155111 };
function populated(nonce, chainId) {
return { return {
type: 2, type: 2,
chainId: 1, chainId: chainId || MAINNET.num,
nonce, nonce,
gasLimit: 100000n, gasLimit: 100000n,
maxFeePerGas: 2000000000n, maxFeePerGas: 2000000000n,
@@ -65,17 +73,17 @@ function populated(nonce) {
}; };
} }
function signedAtNonce(nonce, withWallet) { function signedAtNonce(nonce, withWallet, chainId) {
return (withWallet || signer).signTransaction(populated(nonce)); return (withWallet || signer).signTransaction(populated(nonce, chainId));
} }
// The node the background populates against. Its answers are the numbers the // The node the background populates against. Its answers are the numbers the
// approval screen shows, so they are also the numbers every artifact below is // approval screen shows, so they are also the numbers every artifact below is
// signed at. // signed at.
function fakeProvider(broadcastTransaction, overrides) { function fakeProvider(broadcastTransaction, overrides, chainId) {
return { return {
broadcastTransaction, broadcastTransaction,
getNetwork: async () => Network.from(1), getNetwork: async () => Network.from(chainId || MAINNET.num),
getTransactionCount: async () => NONCE, getTransactionCount: async () => NONCE,
estimateGas: async () => 100000n, estimateGas: async () => 100000n,
getFeeData: async () => ({ getFeeData: async () => ({
@@ -111,14 +119,20 @@ function loadBackground(options) {
const broadcastTransaction = jest.fn(); const broadcastTransaction = jest.fn();
const loadState = jest.fn(opts.loadState || (async () => {})); const loadState = jest.fn(opts.loadState || (async () => {}));
// The network the wallet is on, which the tests switch under a pending
// approval. The node the transaction is populated against is on the same
// one, as it would be: switching networks switches the RPC endpoint too.
let chain = MAINNET;
jest.doMock("../src/shared/state", () => ({ jest.doMock("../src/shared/state", () => ({
state: { rpcUrl: "https://rpc.invalid", wallets: [] }, state: { rpcUrl: "https://rpc.invalid", wallets: [] },
loadState, loadState,
saveState: jest.fn(async () => {}), saveState: jest.fn(async () => {}),
currentNetwork: () => ({ chainId: "0x1" }), currentNetwork: () => ({ chainId: chain.hex }),
})); }));
jest.doMock("../src/shared/balances", () => ({ jest.doMock("../src/shared/balances", () => ({
getProvider: () => fakeProvider(broadcastTransaction, opts.provider), getProvider: () =>
fakeProvider(broadcastTransaction, opts.provider, chain.num),
refreshBalances: jest.fn(async () => {}), refreshBalances: jest.fn(async () => {}),
})); }));
jest.doMock("../src/shared/phishingDomains", () => ({ jest.doMock("../src/shared/phishingDomains", () => ({
@@ -170,7 +184,9 @@ function loadBackground(options) {
getLastFocused: (cb) => cb(null), getLastFocused: (cb) => cb(null),
create: (options2, cb) => { create: (options2, cb) => {
created.push(options2); created.push(options2);
cb({ id: created.length }); // A browser that answers with no window at all. The approval
// then has no window it can ever be answered in.
cb(opts.noWindow ? undefined : { id: created.length });
}, },
remove: (id, cb) => { remove: (id, cb) => {
removed.push(id); removed.push(id);
@@ -204,8 +220,12 @@ function loadBackground(options) {
// Raise a pending transaction approval the way a dApp does, and dig the // Raise a pending transaction approval the way a dApp does, and dig the
// approval id back out of the popup URL the background opened. // approval id back out of the popup URL the background opened.
function requestTx(txParams) { function requestTx(txParams, origin) {
let rpcResult = null; let rpcResult = null;
// The window this request opens, if it opens one. A request refused
// before an approval is raised opens none, and the window belonging to
// some other request must not be handed back as this one's.
const windowIndex = created.length;
const sendResponse = jest.fn((r) => { const sendResponse = jest.fn((r) => {
rpcResult = r; rpcResult = r;
}); });
@@ -215,11 +235,16 @@ function loadBackground(options) {
method: "eth_sendTransaction", method: "eth_sendTransaction",
params: [txParams || TX_PARAMS], params: [txParams || TX_PARAMS],
}, },
{ origin: ORIGIN }, { origin: origin || ORIGIN },
sendResponse, sendResponse,
); );
return { return {
id: () => new URL(created[0].url).searchParams.get("approval"), id: () =>
created.length > windowIndex
? new URL(created[windowIndex].url).searchParams.get(
"approval",
)
: null,
result: () => rpcResult, result: () => rpcResult,
}; };
} }
@@ -269,6 +294,10 @@ function loadBackground(options) {
setActiveAddress: (address) => { setActiveAddress: (address) => {
persisted.activeAddress = address; persisted.activeAddress = address;
}, },
// The user switching network in the toolbar popup.
setNetwork: (network) => {
chain = network;
},
fromPopup: { url: EXT_URL + "src/popup/index.html" }, fromPopup: { url: EXT_URL + "src/popup/index.html" },
}; };
} }
@@ -444,6 +473,318 @@ describe("one approval, one broadcast", () => {
}); });
}); });
// Populating the transaction before the approval window opens is what makes
// the displayed object the verified object. It also fixes the nonce before the
// user has answered anything: two requests populated concurrently take the
// same nonce from a node that has seen neither of them broadcast, and the
// second can then never be sent, because the only way to give it a fresh nonce
// is to populate it again after the user has read the old one off the screen.
// So the second request is refused while the first is unanswered.
describe("one transaction approval at a time", () => {
test("a second eth_sendTransaction while one is pending is refused before it takes a nonce", async () => {
const getTransactionCount = jest.fn(async () => NONCE);
const bg = loadBackground({ provider: { getTransactionCount } });
const first = bg.requestTx();
await settle();
expect(first.id()).toBeTruthy();
expect(getTransactionCount).toHaveBeenCalledTimes(1);
const second = bg.requestTx();
await settle();
expect(second.result()).toEqual({
error: {
code: -32002,
message: expect.stringMatching(
/one transaction at a time.+already in progress/,
),
},
});
// Where the refusal happened matters as much as that it happened: no
// second window, and the node was never asked for a second nonce.
expect(bg.created).toHaveLength(1);
expect(getTransactionCount).toHaveBeenCalledTimes(1);
// The refusal leaves the pending approval untouched, and it still
// sends.
bg.broadcastTransaction.mockResolvedValue({ hash: "0xfeed" });
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id: first.id(),
approved: true,
rawSignedTx: await signedAtNonce(NONCE),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(first.result()).toEqual({ result: "0xfeed" });
});
// The slot is only defensible if the wallet was going to raise an approval
// anyway. Taken any earlier, a request the wallet refuses outright still
// holds it, and any page at all — connected or not — can deny the user's
// own transactions for as long as it keeps asking.
test("a request the wallet refuses does not take the slot from the connected site", async () => {
const bg = loadBackground();
// Both delivered before either reaches its first suspension point,
// which is the interleaving the slot exists for.
const stranger = bg.requestTx(TX_PARAMS, UNCONNECTED_ORIGIN);
const connected = bg.requestTx();
await settle();
expect(stranger.result()).toEqual({
error: { code: 4100, message: "Unauthorized" },
});
// The connected site's transaction was raised, not refused as one the
// user already has in progress.
expect(connected.result()).toBeNull();
expect(connected.id()).toBeTruthy();
expect(bg.created).toHaveLength(1);
});
// The user closes an approval window that looks hung while the attempt
// behind it is still running, and that attempt then fails in a way that
// would normally leave the approval standing for a retry. There is no
// window left to retry in, so leaving it standing answers the requesting
// page never — and holds the slot for the life of the worker with it.
test("an approval whose window closed under a failed attempt is answered, and frees the next request", async () => {
const stalled = deferred();
const bg = loadBackground({
loadState: async () => {
await stalled.promise;
throw new Error("The wallet data could not be read.");
},
});
const first = bg.requestTx();
await settle();
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id: first.id(),
approved: true,
rawSignedTx: await signedAtNonce(NONCE),
},
{ url: bg.fromPopup.url },
);
await settle();
// The attempt owns the approval, so closing the window does not settle
// it: the attempt may yet broadcast, and it is the one that reports.
bg.closeWindow(1);
await settle();
expect(first.result()).toBeNull();
stalled.resolve();
await settle();
expect(first.result()).toEqual({
error: { code: 4001, message: "User rejected the request." },
});
const second = bg.requestTx();
await settle();
expect(second.result()).toBeNull();
expect(second.id()).toBeTruthy();
expect(bg.created).toHaveLength(2);
});
// An approval with no window is one nothing can ever answer.
test("a request whose approval window cannot be opened is answered rather than left waiting", async () => {
const bg = loadBackground({ noWindow: true });
const first = bg.requestTx();
await settle();
expect(first.result()).toEqual({
error: {
code: -32603,
message: expect.stringMatching(
/could not open its approval window/,
),
},
});
// And it did not take the slot with it.
const second = bg.requestTx();
await settle();
expect(second.result()).toEqual({
error: {
code: -32603,
message: expect.stringMatching(
/could not open its approval window/,
),
},
});
});
test("an answered approval frees the next request", async () => {
const bg = loadBackground();
const first = bg.requestTx();
await settle();
// The user closes the approval window, which rejects it.
bg.closeWindow(1);
await settle();
expect(first.result()).toEqual({
error: { code: 4001, message: "User rejected the request." },
});
const second = bg.requestTx();
await settle();
expect(second.id()).toBeTruthy();
expect(bg.created).toHaveLength(2);
});
test("a signature request is not held up by a pending transaction", async () => {
const bg = loadBackground();
bg.requestTx();
await settle();
// A signature consumes no nonce, so it has nothing to collide with.
const signing = bg.requestSign();
await settle();
expect(signing.id()).toBeTruthy();
expect(signing.result()).toBeNull();
expect(bg.created).toHaveLength(2);
});
});
// A nonce collision found before the transaction reaches the network is the
// one send failure the wallet can speak about with certainty. The user is told
// it did not go out and to send it again, rather than being warned it might
// already be on the chain — which would send them looking for a transaction
// that does not exist, and stop them retrying the one that never went.
describe("a nonce collision is reported as a transaction that did not go out", () => {
test("a broadcast the node refused for the nonce is not reported as possibly sent", async () => {
const bg = loadBackground();
const pending = bg.requestTx();
await settle();
bg.broadcastTransaction.mockRejectedValue(
Object.assign(new Error("nonce too low"), {
code: "NONCE_EXPIRED",
}),
);
const answer = bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id: pending.id(),
approved: true,
rawSignedTx: await signedAtNonce(NONCE),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(answer.sendResponse).toHaveBeenCalledWith({
error: expect.stringMatching(/nonce had already been used/),
retryable: false,
stage: "nonce",
});
expect(pending.result()).toEqual({
error: {
message: expect.stringMatching(
/transaction was not sent, because its nonce/,
),
},
});
});
test("a nonce this wallet already broadcast is refused without asking the node again", async () => {
const bg = loadBackground();
const first = bg.requestTx();
await settle();
bg.broadcastTransaction.mockResolvedValue({ hash: "0xfeed" });
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id: first.id(),
approved: true,
rawSignedTx: await signedAtNonce(NONCE),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(first.result()).toEqual({ result: "0xfeed" });
// The stubbed node still reports NONCE as the next nonce — a pending
// count that lags a broadcast the node has already taken — so this
// second approval is populated at a nonce this worker has spent.
const second = bg.requestTx();
await settle();
const answer = bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id: second.id(),
approved: true,
rawSignedTx: await signedAtNonce(NONCE),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
expect(answer.sendResponse).toHaveBeenCalledWith({
error: expect.stringMatching(/nonce had already been used/),
retryable: false,
stage: "nonce",
});
expect(second.result()).toEqual({
error: {
message: expect.stringMatching(/nonce had already been used/),
},
});
});
// Nonce spaces are per chain, and the wallet switches networks. A nonce
// this wallet spent on one chain says nothing about the same nonce on
// another — and low nonces overlap across chains as a matter of course, so
// a record that ignored the chain would refuse ordinary transactions,
// permanently and with a message that is not true of them.
test("a nonce spent on one chain is not refused on another", async () => {
const bg = loadBackground();
const first = bg.requestTx();
await settle();
bg.broadcastTransaction.mockResolvedValue({ hash: "0xfeed" });
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id: first.id(),
approved: true,
rawSignedTx: await signedAtNonce(NONCE),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(first.result()).toEqual({ result: "0xfeed" });
// The user switches network. On this chain the address has sent
// nothing, so the node populates the next transaction at the same
// nonce — correctly.
bg.setNetwork(SEPOLIA);
const second = bg.requestTx();
await settle();
bg.broadcastTransaction.mockResolvedValue({ hash: "0xbeef" });
bg.send(
{
type: "AUTISTMASK_TX_RESPONSE",
id: second.id(),
approved: true,
rawSignedTx: await signedAtNonce(NONCE, undefined, SEPOLIA.num),
},
{ url: bg.fromPopup.url },
);
await settle();
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(2);
expect(second.result()).toEqual({ result: "0xbeef" });
});
});
// The approval carries the transaction the user was shown and the address it // The approval carries the transaction the user was shown and the address it
// was raised for, and the artifact is checked against both. Every case here is // was raised for, and the artifact is checked against both. Every case here is
// one the old comparison — against the dApp's request, for the address that is // one the old comparison — against the dApp's request, for the address that is

View File

@@ -150,7 +150,7 @@ describe("the balance warning on the removal confirmation", () => {
expect(balanceWarningHtml(ETH_ONLY)).toContain("Total: $3,000.00"); expect(balanceWarningHtml(ETH_ONLY)).toContain("Total: $3,000.00");
}); });
// getAddressValueUsd() returns null on testnet and before the first // getAddressValue() reports no value on testnet and before the first
// price fetch. A "Total: $0.00" there would be a lie about the holdings. // 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", () => { test("no USD total is shown when prices are not known", () => {
expect(balanceWarningHtml(TOKEN_ONLY)).not.toContain("Total:"); expect(balanceWarningHtml(TOKEN_ONLY)).not.toContain("Total:");

34
tests/e2e/Dockerfile Normal file
View File

@@ -0,0 +1,34 @@
# Chrome end-to-end image: the pinned Playwright image with this repo and a
# freshly built extension inside it, built by script/test-e2e. The suite is
# still started with `docker run`, so every runtime flag the harness needs
# (--ipc=host in particular) applies as before.
#
# The repo is baked in rather than bind-mounted because a bind mount does
# not resolve under Gitea Actions: the runner runs the job in a container
# against the HOST's docker socket, so the source side of a -v is resolved
# by the host daemon while the job's checkout lives on a docker volume that
# is not a host path -- the mount silently succeeds and /work is empty. A
# build context is streamed to the daemon and so works from anywhere.
# Building the extension here too means the machine starting a run needs
# docker and nothing else.
# mcr.microsoft.com/playwright:v1.56.0-noble, 2026-08-09
#
# The playwright-core devDependency is pinned to the matching Playwright
# version (1.56.0) and the two must be bumped together: the browsers ship
# inside this image, and playwright-core looks for the exact browser
# revision its own version expects. A mismatch fails at launch.
FROM mcr.microsoft.com/playwright@sha256:35246d87a7c88ea9b771c65d33171b2611b02a8253b4b12ce6f94376c55f99f2
WORKDIR /work
# Same layering as the root Dockerfile: script/bootstrap installs the
# prerequisites and the dependencies, and the manifests are copied first so
# that layer is cached until they change.
COPY script/ script/
COPY package.json yarn.lock ./
RUN script/bootstrap
COPY . .
RUN make build

View File

@@ -1,10 +1,24 @@
# Firefox end-to-end image: stock Firefox plus geckodriver on a node base, # 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 # with this repo and a freshly built extension inside it, built by
# harness itself has no dependencies, so nothing is installed for it. # script/test-e2e-firefox. The harness itself has no dependencies, so
# nothing is installed for it.
# #
# All three external artifacts are pinned by digest. The Firefox version in # The build context is the repo root. The repo is baked in rather than
# particular must not float: -remote-allow-system-access is mandatory on 153 # bind-mounted because a bind mount does not resolve under Gitea Actions:
# and was not on 142, so the flag the harness passes is version-coupled. # the runner runs the job in a container against the HOST's docker socket,
# so the source side of a -v is resolved by the host daemon while the job's
# checkout lives on a docker volume that is not a host path -- the mount
# silently succeeds and /work is empty. Baking the build in is also the
# only way this suite can have both a built extension and the
# `--network none` it runs under, since a container with no network cannot
# install anything.
#
# All three external artifacts are pinned by digest, and are fetched in
# layers above the repo copy, so editing the harness or any source file
# re-runs only the two cheap layers at the bottom. 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 # node:22-bookworm-slim, 2026-08-12
FROM node@sha256:d649c27dae7ba0137b3cef5dd75baa422c08dc3d9e3fc0c23dfb172dc3cc6436 FROM node@sha256:d649c27dae7ba0137b3cef5dd75baa422c08dc3d9e3fc0c23dfb172dc3cc6436
@@ -48,4 +62,16 @@ ENV FIREFOX_BIN=/opt/firefox/firefox
ENV GECKODRIVER=/usr/local/bin/geckodriver ENV GECKODRIVER=/usr/local/bin/geckodriver
WORKDIR /work WORKDIR /work
# Same layering as the root Dockerfile: script/bootstrap installs the
# prerequisites and the dependencies, and the manifests are copied first so
# that layer is cached until they change.
COPY script/ script/
COPY package.json yarn.lock ./
RUN script/bootstrap
COPY . .
RUN make build
CMD ["node", "tests/e2e/firefox/run.js", "dist/firefox"] CMD ["node", "tests/e2e/firefox/run.js", "dist/firefox"]

View File

@@ -42,6 +42,7 @@ const {
STUB_TX_HASH, STUB_TX_HASH,
} = require("./network"); } = require("./network");
const { DUST_THRESHOLD_MESSAGE } = require("../../src/popup/dustThreshold"); const { DUST_THRESHOLD_MESSAGE } = require("../../src/popup/dustThreshold");
const { NETWORKS } = require("../../src/shared/networks");
const TEST_TIMEOUT_MS = 120000; const TEST_TIMEOUT_MS = 120000;
@@ -940,6 +941,328 @@ test("confirming removes the address and returns Home (#162)", async (env) => {
); );
}); });
// ------------------------------------------------- Settings screen (#229)
// Settings holds the densest run of $("...") lookups in the codebase, and
// until this section nothing drove it in a browser. One wrong id makes
// settings.init() throw, which aborts the rest of index.js init() before it
// renders anything at all — so a broken id does not degrade Settings, it
// leaves the whole popup blank. These tests assert the controls are there
// AND that they work, because "the view is visible" would still pass
// against a screen whose handlers were never wired.
// The four Token Spam Protection checkboxes, in markup order, with the
// src/shared/state.js key each one is bound to. All four default true.
const SPAM_FILTER_CHECKBOXES = [
{ id: "settings-hide-spoofed-symbols", key: "hideSpoofedSymbols" },
{ id: "settings-hide-low-holders", key: "hideLowHolderTokens" },
{ id: "settings-hide-fraud-contracts", key: "hideFraudContracts" },
{ id: "settings-hide-dust", key: "hideDustTransactions" },
];
// The one toggled through a reopen. Chosen because nothing later in this
// suite depends on it: the other three filter token and transaction lists
// that the ConfirmTx and dApp sections go on to drive.
const TOGGLED_FILTER = "settings-hide-dust";
// Everything the Settings assertions below must observe, recorded as each
// group of them completes. The final test demands the exact set.
//
// The point is that a green run cannot mean the assertions were skipped.
// Navigation that silently fails already fails a test — visible() throws
// on a timeout — but an early return, a deleted test, or a body that
// stopped being reached would otherwise shrink this section quietly
// instead of reddening the run.
const SETTINGS_COVERAGE = [
"about-well",
"spam-checkbox-defaults",
"theme-select",
"network-select",
"selector-round-trip",
"selector-restore",
"toggle-off-survives-reopen",
"toggle-on-survives-reopen",
"wallet-list",
];
// A control read as the DOM has it, not as a selector claims: tag name and
// type distinguish a real <input type="checkbox"> from a <div> that merely
// carries the id, and `checked` is the live property rather than the
// attribute, so it reflects what init() assigned.
function controlState(page, id) {
return page.evaluate((elementId) => {
const el = document.getElementById(elementId);
if (!el) return null;
return {
tag: el.tagName.toLowerCase(),
type: el.type || "",
checked: el.checked,
value: el.value,
options: Array.from(el.options || []).map((o) => o.value),
};
}, id);
}
async function checkboxStates(page) {
const out = {};
for (const { id } of SPAM_FILTER_CHECKBOXES) {
out[id] = await controlState(page, id);
}
return out;
}
function assertSpamCheckbox(st, id, expected, where) {
assert(st !== null, "no element with id " + id + " on Settings " + where);
assert(
st.tag === "input" && st.type === "checkbox",
id + " is a <" + st.tag + " type=" + st.type + ">, not a checkbox",
);
assert(
st.checked === expected,
id +
" reads " +
st.checked +
" " +
where +
", expected " +
expected +
" — the checkbox is on screen but not carrying the persisted value",
);
}
test("Settings renders with the whole screen populated (#229)", async (env) => {
await visible(env.page, "#view-main");
await openSettings(env.page);
// show() writes the About well near its end — only the debug well and
// the debug-mode checkbox follow it — and showView() is the last thing
// of all, so an id show() cannot find aborts before Settings is ever
// displayed. Reading these values back proves show() ran through to
// there, not just far enough to unhide the section. They are filled
// from build-time constants that always have a value, so empty means
// the write did not happen.
const about = await env.page.evaluate(() => {
const out = {};
for (const id of [
"about-license",
"about-author",
"about-version",
"about-release-date",
"about-commit-link",
]) {
const el = document.getElementById(id);
out[id] = el === null ? null : el.textContent.trim();
}
return out;
});
for (const [id, text] of Object.entries(about)) {
assert(
text !== null && text.length > 0,
"the About well left #" +
id +
" unwritten: " +
JSON.stringify(about),
);
}
env.settingsCoverage.add("about-well");
// The wallet list is rendered by settings.js rather than authored in
// index.html, so an empty container means renderWalletListSettings()
// did not run even though the screen came up.
const wallets = await env.page
.locator("#settings-wallet-list .settings-wallet-name")
.count();
assert(
wallets >= 2,
"Settings lists " +
wallets +
" wallets; the suite created two by this point",
);
env.settingsCoverage.add("wallet-list");
});
test("the four Token Spam Protection checkboxes render, defaulted on (#229)", async (env) => {
await openSettings(env.page);
const states = await checkboxStates(env.page);
for (const { id } of SPAM_FILTER_CHECKBOXES) {
assertSpamCheckbox(states[id], id, true, "on first render");
}
env.settingsCoverage.add("spam-checkbox-defaults");
});
test("the theme and network selectors render their real choices (#229)", async (env) => {
await openSettings(env.page);
const theme = await controlState(env.page, "settings-theme");
assert(theme !== null, "no #settings-theme element on Settings");
assert(
theme.tag === "select",
"#settings-theme is a <" + theme.tag + ">, not a <select>",
);
assert(
theme.options.join(",") === "system,light,dark",
"the theme selector offers " + JSON.stringify(theme.options),
);
env.settingsCoverage.add("theme-select");
const network = await controlState(env.page, "settings-network");
assert(network !== null, "no #settings-network element on Settings");
assert(
network.tag === "select",
"#settings-network is a <" + network.tag + ">, not a <select>",
);
const wantNetworks = Object.keys(NETWORKS).sort().join(",");
assert(
network.options.slice().sort().join(",") === wantNetworks,
"the network selector offers " +
JSON.stringify(network.options) +
", expected the networks in src/shared/networks.js: " +
wantNetworks,
);
env.settingsCoverage.add("network-select");
});
// Reads both selectors in one page task, so a round trip cannot observe
// them at two different moments.
async function selectorValues(page) {
const theme = await controlState(page, "settings-theme");
const network = await controlState(page, "settings-network");
assert(theme !== null, "no #settings-theme element on Settings");
assert(network !== null, "no #settings-network element on Settings");
return { theme: theme.value, network: network.value };
}
function assertSelectors(got, wantTheme, wantNetwork, where) {
assert(
got.theme === wantTheme,
"the theme selector shows " +
JSON.stringify(got.theme) +
" " +
where +
", expected " +
JSON.stringify(wantTheme),
);
assert(
got.network === wantNetwork,
"the network selector shows " +
JSON.stringify(got.network) +
" " +
where +
", expected " +
JSON.stringify(wantNetwork),
);
}
// The two values the selectors are driven to. NEITHER is the first
// <option> of its <select> (`system` and `mainnet` are), and that is the
// entire point: the first option is what the DOM reports with no
// JavaScript involved at all, so asserting it would pass just as happily
// against a Settings screen that never assigned anything. Only a value
// that went out through the change handler and saveState(), and came back
// through loadState() and the assignment show()/init() makes, can be read
// here.
const NONDEFAULT_THEME = "dark";
const NONDEFAULT_NETWORK = "sepolia";
test("the theme and network selectors carry a non-default persisted value (#229)", async (env) => {
await openSettings(env.page);
// selectOption() fires "change", which is what the handlers bind.
await env.page.selectOption("#settings-theme", NONDEFAULT_THEME);
await env.page.selectOption("#settings-network", NONDEFAULT_NETWORK);
await reopenPopup(env, "#view-settings");
assertSelectors(
await selectorValues(env.page),
NONDEFAULT_THEME,
NONDEFAULT_NETWORK,
"after reopening the popup",
);
env.settingsCoverage.add("selector-round-trip");
// Restore, the same way round, and assert the restore actually took
// rather than trusting it: the later sections inherit this fixture,
// and a selector stuck on `dark`/`sepolia` would otherwise be
// indistinguishable here from one that persists correctly. Switching
// the network back also returns state.rpcUrl and state.blockscoutUrl
// to the mainnet defaults that onChainSwitch() overwrote, which are
// the values src/shared/state.js starts with.
await env.page.selectOption("#settings-theme", "system");
await env.page.selectOption("#settings-network", "mainnet");
await reopenPopup(env, "#view-settings");
assertSelectors(
await selectorValues(env.page),
"system",
"mainnet",
"after restoring and reopening the popup",
);
env.settingsCoverage.add("selector-restore");
});
// The functional half. A checkbox that renders but is not wired looks
// identical on screen; only a value that survives being written to storage
// and read back by a fresh page load tells the two apart. That round trip
// runs through the change handler, saveState(), loadState() and the
// assignment init() makes — every part of the wiring at once.
test("a spam filter toggled in Settings survives a popup reopen (#229)", async (env) => {
await openSettings(env.page);
await env.page.click("#" + TOGGLED_FILTER);
const immediately = await controlState(env.page, TOGGLED_FILTER);
assert(
immediately.checked === false,
"clicking #" + TOGGLED_FILTER + " did not clear it",
);
await reopenPopup(env, "#view-settings");
const after = await checkboxStates(env.page);
for (const { id } of SPAM_FILTER_CHECKBOXES) {
assertSpamCheckbox(
after[id],
id,
id !== TOGGLED_FILTER,
"after reopening the popup",
);
}
env.settingsCoverage.add("toggle-off-survives-reopen");
});
test("turning the same filter back on survives a reopen too (#229)", async (env) => {
await openSettings(env.page);
await env.page.click("#" + TOGGLED_FILTER);
await reopenPopup(env, "#view-settings");
// Restores the fixture the later sections inherit, and rules out a
// checkbox that persists "off" only because it is stuck there.
const after = await checkboxStates(env.page);
for (const { id } of SPAM_FILTER_CHECKBOXES) {
assertSpamCheckbox(after[id], id, true, "after toggling back on");
}
env.settingsCoverage.add("toggle-on-survives-reopen");
await env.page.click("#btn-settings-back");
await visible(env.page, "#view-main");
});
test("the Settings assertions above all ran (#229)", async (env) => {
const seen = [...env.settingsCoverage].sort();
const want = SETTINGS_COVERAGE.slice().sort();
assert(
seen.join(",") === want.join(","),
"the Settings section covered " +
JSON.stringify(seen) +
" but must cover " +
JSON.stringify(want) +
" — a green run here would otherwise mean only that fewer " +
"assertions ran, not that they passed",
);
});
// ------------------------------------------------ dust threshold (#233) // ------------------------------------------------ dust threshold (#233)
// The popup size README documents the UI as designed for. Pages in this // The popup size README documents the UI as designed for. Pages in this
@@ -2668,6 +2991,11 @@ async function main() {
// The recovery phrase of the wallet created in test 2, so later // The recovery phrase of the wallet created in test 2, so later
// tests can assert on the real secret rather than its shape. // tests can assert on the real secret rather than its shape.
phrase: null, phrase: null,
// What the Settings section (#229) actually observed. A guard test
// at the end of that section demands the full set, so a skipped or
// silently shortened assertion reddens the run instead of shrinking
// it.
settingsCoverage: new Set(),
// Confirmation-screen heights, measured in the pending state and // Confirmation-screen heights, measured in the pending state and
// compared against every later state of the same screen. // compared against every later state of the same screen.
ethPendingHeight: null, ethPendingHeight: null,

View File

@@ -0,0 +1,182 @@
// Every element id the popup views look up must exist in the markup they
// look it up in.
//
// The failure this catches: `$("settings-hide-dsut")` is valid JavaScript
// referring to a defined function, so neither jest (node environment, no
// DOM) nor a linter has anything to object to. At runtime `$()` returns
// null and the next property access throws, which in `init()` aborts the
// rest of that view's wiring and takes the whole screen down. Settings is
// the densest concentration of these lookups in the codebase.
//
// This is the cheap general half of the guard: it runs in `make check`
// with no browser and covers every id in every view, not the ones some
// test happens to click. The expensive specific half is the Settings
// section of the end-to-end suite (tests/e2e/run.js), which proves the
// screen actually comes up and its controls work.
//
// Scope and limits, stated rather than implied:
// - Only literal string arguments are resolvable statically. A call
// like `$(containerId)` is invisible here; those are covered by the
// e2e run instead.
// - `document.getElementById()` is checked too, minus the ids listed in
// RUNTIME_CREATED_IDS, which name nodes the code creates itself and
// which are legitimately absent from the static markup.
"use strict";
const fs = require("fs");
const path = require("path");
const POPUP_DIR = path.join(__dirname, "..", "src", "popup");
const POPUP_HTML_PATH = path.join(POPUP_DIR, "index.html");
// Nodes built at runtime rather than authored in index.html. Each one must
// be created unconditionally by the code before it is ever looked up.
const RUNTIME_CREATED_IDS = new Set([
// Created by updateDebugBanner() in src/popup/views/helpers.js.
"debug-banner",
]);
// Every id lookup the popup performs with a literal argument, as
// {id, file, line, source} records.
//
// showView("x") is included because it resolves to the element id
// "view-x": a view name with no matching section is the same defect one
// indirection further out.
const PATTERNS = [
{ re: /\$\(\s*"([^"\n]+)"\s*\)/g, id: (m) => m[1], source: "$()" },
{
re: /document\.getElementById\(\s*"([^"\n]+)"\s*\)/g,
id: (m) => m[1],
source: "getElementById()",
},
{
re: /\b(?:showError|hideError)\(\s*"([^"\n]+)"/g,
id: (m) => m[1],
source: "showError()/hideError()",
},
{
re: /\bshowView\(\s*"([^"\n]+)"\s*\)/g,
id: (m) => "view-" + m[1],
source: "showView()",
},
];
function jsFilesUnder(dir) {
const out = [];
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
const full = path.join(dir, entry.name);
if (entry.isDirectory()) {
out.push(...jsFilesUnder(full));
} else if (entry.name.endsWith(".js")) {
out.push(full);
}
}
return out.sort();
}
function lineOf(text, index) {
return text.slice(0, index).split("\n").length;
}
function collectReferences() {
const refs = [];
for (const file of jsFilesUnder(POPUP_DIR)) {
const text = fs.readFileSync(file, "utf8");
const rel = path.relative(path.join(__dirname, ".."), file);
for (const { re, id, source } of PATTERNS) {
re.lastIndex = 0;
let m;
while ((m = re.exec(text)) !== null) {
refs.push({
id: id(m),
file: rel,
line: lineOf(text, m.index),
source,
});
}
}
}
return refs;
}
function collectHtmlIds(html) {
const ids = [];
const re = /\bid="([^"]+)"/g;
let m;
while ((m = re.exec(html)) !== null) ids.push(m[1]);
return ids;
}
const HTML = fs.readFileSync(POPUP_HTML_PATH, "utf8");
const HTML_IDS = collectHtmlIds(HTML);
const HTML_ID_SET = new Set(HTML_IDS);
const REFERENCES = collectReferences();
describe("every element id the popup looks up exists in its markup", () => {
// A guard that found nothing to check would pass forever. If a
// refactor renames the directory, changes the helper, or moves the
// markup, this fails instead of quietly covering zero call sites.
// The floors are far below the counts measured when this was written
// (434 lookups across 20 of the 24 files under src/popup/, against 274
// ids in the markup), so ordinary churn does not trip them.
test("the scan actually found the code and the markup", () => {
const files = new Set(REFERENCES.map((r) => r.file));
expect(files.size).toBeGreaterThanOrEqual(15);
expect(REFERENCES.length).toBeGreaterThanOrEqual(300);
expect(HTML_IDS.length).toBeGreaterThanOrEqual(200);
// The densest screen, named explicitly: a scan that stopped
// covering src/popup/views/settings.js is the exact regression
// this file was written for.
expect(
files.has(path.join("src", "popup", "views", "settings.js")),
).toBe(true);
expect(
REFERENCES.some((r) => r.id === "settings-hide-spoofed-symbols"),
).toBe(true);
expect(REFERENCES.some((r) => r.id === "view-settings")).toBe(true);
});
test("no lookup names an id that src/popup/index.html does not define", () => {
const missing = REFERENCES.filter(
(r) => !HTML_ID_SET.has(r.id) && !RUNTIME_CREATED_IDS.has(r.id),
).map(
(r) =>
r.file +
":" +
r.line +
" " +
r.source +
' looks up id "' +
r.id +
'", which is not in src/popup/index.html',
);
expect(missing).toEqual([]);
});
test("every id excused as runtime-created is still looked up somewhere", () => {
// Otherwise the exception list becomes a place stale names
// accumulate, and the next real miss can be waved through by
// adding one more.
for (const id of RUNTIME_CREATED_IDS) {
expect(REFERENCES.some((r) => r.id === id)).toBe(true);
expect(HTML_ID_SET.has(id)).toBe(false);
}
});
test("index.html defines no id twice", () => {
// getElementById returns the first match, so a duplicate id means
// one of the two elements can never be reached by the code that
// thinks it owns it.
const seen = new Set();
const duplicated = [];
for (const id of HTML_IDS) {
if (seen.has(id)) duplicated.push(id);
seen.add(id);
}
expect(duplicated).toEqual([]);
});
});