Compare commits

..

3 Commits

Author SHA1 Message Date
6a01688106 fix: floor the persisted fields a restore dereferences, and make each field's floor an executable claim (closes #362)
All checks were successful
check / check (push) Successful in 40s
e2e / e2e-chrome (push) Successful in 1m45s
e2e / e2e-firefox (push) Successful in 32s
A persisted container was checked while its ENTRIES were dereferenced
unchecked. A stored `{"0x…": "notalist"}` in allowedSites passes the state
gate, renders a working popup, and then throws inside saveState()'s per-
hostname merge, so every save from that moment on fails while the UI looks
entirely healthy. deniedSites has the identical shape; fraudContracts is the
same class with a milder consequence.

The sweep for that class found four more:

- selectedToken, dereferenced as text behind a truthiness-only restore gate.
- rpcUrl, handed whole to `new JsonRpcProvider()` by getProvider(), which
  throws SYNCHRONOUSLY for a non-string — from txStatus.js and addWallet.js,
  neither inside a try, and the first reachable from a stored
  `currentView: "wait-tx"` through the unguarded restoreView().
- The ENTRIES of viewData. Four restore branches gate on one truthy field and
  hand the rest to a renderer that calls address.toLowerCase(): a stored
  `{"currentView":"success-tx","viewData":{"hash":"0x1"}}` throws out of
  restoreView(), skipping the rest of popup init.
- selectedWallet / selectedAddress. `wallets` is a real Array, so a stored
  "map", "length", "constructor" or "__proto__" is TRUTHY: hasValidAddress()'s
  `&&` does not short-circuit and `.addresses[…]` throws. A stale INTEGER index
  is the safe case.

Floors, in src/shared/persistedState.js: allowedSites/deniedSites through
siteMap(), fraudContracts and each hostname list through textList(),
selectedToken and activeAddress as text-or-null, rpcUrl and blockscoutUrl as
non-empty text, selectedWallet and selectedAddress as a non-negative integer
or null, and each networkEndpoints pair's two URL fields — which
applyChainSwitchFields() assigns straight onto s.rpcUrl on the next switch.

Guards, in src/popup/viewRouter.js: the four restore branches that gate on one
truthy field now check the entries their renderer dereferences, as
txStatus.restoreWait() has always done for wait-tx. "confirm-tx" joins
ADDRESS_VIEWS, because its Sign button dereferences
state.wallets[state.selectedWallet] behind no guard of its own.

A stored own "__proto__" key is dropped by siteMap(): it can never be a wallet
address, so it grants nothing, and keeping it only keeps a value the next save
would hand to the prototype setter. networkEndpoints keeps unknown keys by
design, so mergeMapByKey() in src/shared/state.js now writes with
defineProperty as well — the guard in the floor was being undone one layer
downstream.

A save that fails is also told, not merely repaired: onSaveFailure() reports
every failed save, awaited or not (the save queue's own rejection handler is
what made a failure vanish), and the popup raises a persistent "NOT SAVED"
banner naming the reason. doRefreshAndRender() no longer rejects, since every
one of its call sites fires it and walks away.

The per-field justification in the header of src/shared/stateSchema.js is
replaced by tests/persistedFieldContract.test.js. That comment shipped a false
claim in three consecutive changes; the artifact was the problem. The test is
one row per persisted field, declaring the property that field's floor is
claimed to have and PROVING it by driving the real code with hostile values —
the gate for a field the gate refuses, normalizePersisted() for a field it
floors, the real JsonRpcProvider constructor for rpcUrl, and — for every field
whose only defence is that nothing dereferences it structurally — a boot of the
real popup entry point over that value onto EVERY view the popup can reopen
onto.

That last part is what makes the claim falsifiable, and it is why this defect
class is worth a harness at all: it lives on the RESTORE path and not on Home.
So the suite goes red on a field any restorable view dereferences on render, on
a field that gains a floor while its row still claims it has none, and on a
field added to PERSISTED_FIELDS with no row. What it does not reach is what no
stored record reaches by itself: a view only forward navigation opens, and
anything behind a click. The header and the README mirror now point at it
instead of restating it.

The boots are cheap enough to keep by construction rather than by sampling.
Every field the router itself reads is driven onto each view individually,
since a hostile value in one of those legitimately changes which view renders;
every other unfloored field is corrupted on the SAME boot, and that boot has to
land on the view it stored — so a field that does move the routing cannot hide
in the crowd, and the failure path re-boots one field at a time to name it.
That is thirty-three boots instead of six hundred; the suite runs in about 13s
against a 30s cap.

The DOM stub in tests/support/popupBoot.js gained one thing to make any of that
possible: an element's parentElement. Without it success-tx and transaction
threw on the first line that hides a field's wrapper, so neither renderer could
be booted onto at all — every boot aimed at them fell back to Home instead, and
the base profile the sweep starts from is now asserted to render each view
rather than fall back, so that cannot go unnoticed again.
2026-08-23 19:37:25 +00:00
45500e66cf fix: declare and ship toolbar icons, so neither browser renders a puzzle piece (closes #371)
All checks were successful
check / check (push) Successful in 33s
e2e / e2e-chrome (push) Successful in 1m45s
e2e / e2e-firefox (push) Successful in 32s
Neither manifest declared any icons, so both browsers showed a generic puzzle-piece -- the first thing seen on every browser start, and how a user tells a real extension from a look-alike. Both manifests now declare 16/32/48/128, and the PNGs ship inside each browser archive rather than being left at dist/ root, which is the trap that made a naive zip incomplete before.

build.js reads which icons to copy from each manifest's own icons block, so the manifest is the single source of truth and a declared-but-absent size fails the build rather than shipping a dangling reference; the packager's reference-resolver covers them independently. Manifest values are constrained before being joined into a path. The artwork is original, generated from geometry rather than traced or fetched.
2026-08-23 21:26:14 +02:00
1b52aa1723 fix: store an absent explorer decimals as unknown instead of fabricating 18 (closes #349)
All checks were successful
check / check (push) Successful in 34s
e2e / e2e-chrome (push) Successful in 1m45s
e2e / e2e-firefox (push) Successful in 30s
parseInt(decimals || "18") ran before writing stored tokenBalances[].decimals, so an explorer reporting no decimals produced a fabricated 18 indistinguishable from a real one at read time. That defeated the resolve-or-refuse guarantees of #306 and #340: their refusal paths were intact but never fired, because the guess was laundered upstream of them.

An absent scale is now stored as unknown, and a holding whose scale nothing knows carries a null balance -- unknown, never zero -- with six reader sites saying so rather than printing 0.0000. The Send screen resolves the display scale rather than reading the stored one, so a bundled token whose explorer row omits decimals still sends; when the scale cannot be resolved the stored quantity is withdrawn too, so the user is told the balance is unknown rather than only that the fee failed.

Existing fabricated 18s cannot be told apart retroactively and are replaced wholesale on the next balance refresh. An explorer-sourced scale stays trusted -- only fabrication is removed; the reasoning is recorded on the issue.
2026-08-23 21:19:04 +02:00
27 changed files with 1707 additions and 220 deletions

View File

@@ -902,6 +902,27 @@ the swap's `Amount` and `Min. received` lines (`src/shared/uniswap.js`). An
unbounded allowance or permit needs no scale to describe and is still shown as
`Unlimited`.
The rule holds only if nothing invents a scale UPSTREAM of it. Those three
sources are read as authoritative, so a value written into one of them cannot be
recognized as a guess afterwards: a fabricated `18` reads exactly like a real
`18`, and the refusal above then never fires. So `fetchTokenBalances()` in
`src/shared/balances.js` stores what the explorer reported or `null`, never a
default, and the same holds for the history list's token transfers in
`src/shared/transactions.js`. A token whose `decimals()` reverts has no scale
anywhere, and a holding of it carries no quantity either: its balance is `null`
— read as unknown, never as zero — and the balance list says so rather than
printing `0.0000` for money that is really there. `0` is a real scale and is
never treated as absent.
`tokenBalances[].decimals` is therefore the explorer's own answer and nothing
else, which is not the same question as the scale a screen should render at.
Anything that needs the second one calls `resolveTokenDecimals()` — the balance
list, the approval and swap lines, and the Send screen, which carries the
resolved scale onto the pending transaction for `transferAmount.js` to encode
and compare against. Reading the stored field directly instead answers `null`
for a bundled or tracked token the explorer merely omitted, which is not a
refusal the wallet has any reason to make.
#### Partial USD totals
Prices are fetched for the top 25 tokens only, so an address can hold assets the
@@ -1027,10 +1048,16 @@ because nothing dereferences them structurally.
Which field is which is not written in prose anywhere, deliberately.
`tests/persistedFieldContract.test.js` is the list: one row per persisted field,
naming the property that field's floor is claimed to have and proving it by
driving the real code with hostile values — including a boot of the real popup
entry point for every field whose only defence is that nothing dereferences it.
A field added to `PERSISTED_FIELDS` with no row fails `make check`, and so does
a row whose claim is false. The per-field justification that used to live in the
driving the real code with hostile values — and, for every field whose only
defence is that nothing dereferences it, by booting the real popup entry point
over that value onto every view the popup can reopen onto. That last part is
what makes the claim falsifiable, because this defect class lives on the restore
path rather than on the home screen: a field that gains a structural dereference
in any restorable view's render fails `make check`, as does a field that gains a
floor while its row still claims it has none, and as does a field added to
`PERSISTED_FIELDS` with no row at all. What the boot does not reach is what no
stored record reaches by itself — a view only forward navigation opens, and
anything behind a click. The per-field justification that used to live in the
header of `src/shared/stateSchema.js` shipped a false claim in three consecutive
changes, each caught only by a reviewer re-deriving thirty fields by hand.

60
TODO.md
View File

@@ -45,6 +45,18 @@ but the review is broader than any of them.
# Completed Steps
- 2026-08-23: Both manifests declare toolbar icons, and real PNGs at
16/32/48/128 ship inside both archives
([#371](https://git.eeqj.de/sneak/AutistMask/issues/371)). Neither manifest
had an `icons` block, so both browsers drew a generic puzzle piece — the first
thing the owner sees on every launch, and how a user tells a real extension
from a look-alike. The sizes `build.js` copies into each browser directory are
read out of the manifest that ships next to them rather than from a second
list, so a declared size `icons/` does not hold fails `make build`;
`script/lib/package.js` already resolves `.png` references, so an icon that
reached a manifest but not the archive fails packaging. The artwork is
original: a flat dark-navy rounded field with a teal triangular "A", drawn
from geometry and rasterised into PNG, nothing traced or downloaded.
- 2026-08-23: A persisted container whose ENTRIES were dereferenced unchecked no
longer reaches a `.map()` or a `.toLowerCase()`
([#362](https://git.eeqj.de/sneak/AutistMask/issues/362)). `allowedSites` was
@@ -67,8 +79,12 @@ but the review is broader than any of them.
justification in the header of `src/shared/stateSchema.js` — which had shipped
a false claim in three consecutive changes — is replaced by
`tests/persistedFieldContract.test.js`, one row per persisted field, each
proven by driving the real code with hostile values; a field with no row, or a
row whose claim is false, now fails `make check`.
proven by driving the real code with hostile values — and, for a field whose
only defence is that nothing dereferences it, by booting the real popup entry
point over that value onto every view the popup can reopen onto, since that is
the path this whole class of defect lives on. A field with no row, a field
that gains a floor while its row still claims it has none, and a field any
restorable view dereferences on render now all fail `make check`.
- 2026-08-23: A swap amount and the token it is counted in now always come from
the same hop, on both sides of the approval screen
([#359](https://git.eeqj.de/sneak/AutistMask/issues/359) and
@@ -146,6 +162,46 @@ but the review is broader than any of them.
`src/shared/restorableViews.js`, since `persistedState.js` requires it and
that module is in the background bundle.
- 2026-08-23: An explorer that reports no `decimals` for a token no longer has a
scale invented for it before storage
([#349](https://git.eeqj.de/sneak/AutistMask/issues/349)).
`fetchTokenBalances()` did `parseInt(item.token.decimals || "18", 10)` on the
way in, so a token whose `decimals()` reverts was written to
`tokenBalances[].decimals` as a fabricated `18` that no reader could tell from
a real one. That is upstream of the resolve-or-refuse rule
([#306](https://git.eeqj.de/sneak/AutistMask/issues/306),
[#340](https://git.eeqj.de/sneak/AutistMask/issues/340)): both approval paths
read this stored value as an authoritative source, so the guess walked past
refusals that were intact and simply never fired. The stored value is now the
explorer's own answer or `null`, and both the ERC-20 amount line and the swap
lines reach `unknownDecimalsAmount()` on it. The history list's token
transfers carried the same `|| "18"` and now state base units with the scale
unknown rather than a quantity. A holding whose scale nothing knows carries
`balance: null` — unknown, not zero — and the balance list, the USD total, the
Send screen and the confirmation screen each say so instead of printing
`0.0000` for money that is really there. The uint8 check is one shared
`toDecimals()` rather than three copies, and it answers `0` for a real scale
of zero: `|| "18"` collapsed that to eighteen, the trap of
[#246](https://git.eeqj.de/sneak/AutistMask/issues/246). Existing installs
hold `18`s that cannot be told apart retroactively; they display exactly as
they do today until the next balance refresh, which rewrites `tokenBalances`
wholesale and needs no user action. No `|| 18` or `?? 18` fallback remains
anywhere in `src/`; the literal `18`s that do remain are real data, not
defaults — 432 per-token `decimals: 18` entries in the bundled
`src/shared/tokenList.js`, and, outside that file, only native ETH's
protocol-defined scale in `src/shared/uniswap.js` and the fixed-point
comparison scale in `src/shared/txValidation.js`. `tokenBalances[].decimals`
is the explorer's answer alone and not the scale a screen renders at, so the
Send screen resolves through `resolveTokenDecimals()` like every other
consumer: reading the stored field raw carried a `null` into `estimateGas()`
for a bundled token such as WETH, which reported an unestimable network fee
and left Send disabled behind a message no retry could clear. Send resolves
with `wallets`, which adds the cross-address disagreement check the balance
list does not make, so the two can differ; where they do, the stored quantity
was computed at a scale Send has refused, and it is withdrawn with it. An
unknown scale is an unknown balance, and the user is told that rather than
that the fee could not be estimated.
- 2026-08-23: The background no longer reads or writes the shared `state`
singleton ([#324](https://git.eeqj.de/sneak/AutistMask/issues/324)), which
also closes the cold-worker wrong-chain send

View File

@@ -51,6 +51,17 @@ const RECEIPT_ENV = "AUTISTMASK_BUILD_RECEIPT";
// rather than writing a receipt that cannot be checked.
const SAFE_EMITTED_PATH = /^dist\/[A-Za-z0-9._][A-Za-z0-9._/-]*$/;
// Where each browser directory's manifest comes from, and — through its
// "icons" — which image files ship inside that directory.
const MANIFEST_SOURCES = new Map([
[DIST_CHROME, path.join(__dirname, "manifest", "chrome.json")],
[DIST_FIREFOX, path.join(__dirname, "manifest", "firefox.json")],
]);
// What an "icons" entry may name: a plain file under icons/, so a manifest
// value is never joined into a path that leaves the repo.
const ICON_REF_RE = /^icons\/[A-Za-z0-9._-]+\.png$/;
function ensureDir(dir) {
fs.mkdirSync(dir, { recursive: true });
}
@@ -257,6 +268,42 @@ function copyEmitted(src, dest) {
recordEmitted(dest);
}
// Copy the icons one browser directory ships. The sizes come from the manifest
// that will sit next to them, not from a second list here: a size the manifest
// declares and icons/ does not hold fails the build, rather than shipping a
// manifest whose reference resolves to nothing. Relative to the browser
// directory, so nothing points up and out of it the way dist/styles.css does.
function copyIcons(distDir) {
const manifestPath = MANIFEST_SOURCES.get(distDir);
const manifest = JSON.parse(fs.readFileSync(manifestPath, "utf8"));
const refs = Object.values(manifest.icons || {});
if (refs.length === 0) {
throw new Error(
`${repoRelative(manifestPath)} declares no icons, so the browser ` +
`renders a generic placeholder for this extension`,
);
}
for (const ref of refs) {
if (!ICON_REF_RE.test(ref)) {
throw new Error(
`${repoRelative(manifestPath)} declares icon ` +
`${JSON.stringify(ref)}, which is not a plain file under ` +
`icons/`,
);
}
const src = path.join(__dirname, ref);
if (!fs.existsSync(src)) {
throw new Error(
`${repoRelative(manifestPath)} declares ${ref}, which is not ` +
`in this tree`,
);
}
const dest = path.join(distDir, ref);
ensureDir(path.dirname(dest));
copyEmitted(src, dest);
}
}
function sha256File(absPath) {
return crypto
.createHash("sha256")
@@ -524,6 +571,8 @@ async function build() {
tailwindOutput,
path.join(distDir, "src", "popup", "styles.css"),
);
copyIcons(distDir);
}
// copy manifests

BIN
icons/icon128.png Normal file

Binary file not shown.

After

Width:  |  Height:  |  Size: 1.8 KiB

BIN
icons/icon16.png Normal file

Binary file not shown.

After

Width:  |  Height:  |  Size: 292 B

BIN
icons/icon32.png Normal file

Binary file not shown.

After

Width:  |  Height:  |  Size: 534 B

BIN
icons/icon48.png Normal file

Binary file not shown.

After

Width:  |  Height:  |  Size: 725 B

View File

@@ -9,6 +9,12 @@
"content_security_policy": {
"extension_pages": "default-src 'self'; script-src 'self' 'wasm-unsafe-eval'; object-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self' https: http:; frame-src 'none'; form-action 'none'; base-uri 'none'"
},
"icons": {
"16": "icons/icon16.png",
"32": "icons/icon32.png",
"48": "icons/icon48.png",
"128": "icons/icon128.png"
},
"action": {
"default_popup": "src/popup/index.html"
},

View File

@@ -5,6 +5,12 @@
"description": "Minimal Ethereum wallet for Firefox",
"permissions": ["storage", "activeTab", "alarms", "<all_urls>"],
"content_security_policy": "default-src 'self'; script-src 'self' 'wasm-unsafe-eval'; object-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; connect-src 'self' https: http:; frame-src 'none'; form-action 'none'; base-uri 'none'",
"icons": {
"16": "icons/icon16.png",
"32": "icons/icon32.png",
"48": "icons/icon48.png",
"128": "icons/icon128.png"
},
"browser_action": {
"default_popup": "src/popup/index.html"
},

View File

@@ -12,6 +12,7 @@ const {
displaySymbol,
truncateMiddle,
balanceLine,
unknownableAmount,
renderAddressHtml,
attachCopyHandlers,
goBack,
@@ -118,7 +119,9 @@ function show() {
addr.tokenBalances,
state.trackedTokens,
);
amount = tb ? parseFloat(tb.balance || "0") : 0;
// null when the scale is unknown: no quantity to show, and none to
// price. balanceLine() states that rather than printing 0.0000.
amount = tb ? unknownableAmount(tb.balance) : 0;
price = getPrice(symbol);
}
@@ -152,7 +155,7 @@ function show() {
attachCopyHandlers($("address-token-line"));
// USD total for this token only
const usdVal = price ? amount * price : null;
const usdVal = price && amount !== null ? amount * price : null;
const usdStr = formatUsd(usdVal);
$("address-token-usd-total").innerHTML = usdStr || "&nbsp;";

View File

@@ -139,12 +139,17 @@ function show(txInfo) {
// Balance (with inline USD)
if (isErc20) {
const bal = txInfo.tokenBalance || "0";
const balUsd = tokenPrice ? parseFloat(bal) * tokenPrice : null;
$("confirm-balance").textContent = valueWithUsd(
bal + " " + symbol,
balUsd,
);
// null is a balance whose scale nothing knows, not a balance of zero
// (https://git.eeqj.de/sneak/AutistMask/issues/349). The send is
// refused at encode time for the same missing scale; what this line
// must not do is state a quantity nobody established.
const bal = txInfo.tokenBalance;
const balUsd =
tokenPrice && bal != null ? parseFloat(bal) * tokenPrice : null;
$("confirm-balance").textContent =
bal == null
? "unknown (" + symbol + ")"
: valueWithUsd(bal + " " + symbol, balUsd);
} else {
const bal = txInfo.balance || "0";
const balUsd = ethPrice ? parseFloat(bal) * ethPrice : null;
@@ -235,17 +240,22 @@ function renderValidation(txInfo) {
}
if (codes.includes(CODES.INSUFFICIENT_TOKEN)) {
messages.push(
"Insufficient " +
symbol +
" balance. You have " +
txInfo.tokenBalance +
" " +
symbol +
" but are trying to send " +
txInfo.amount +
" " +
symbol +
".",
txInfo.tokenBalance == null
? "This token's balance is unknown, because nothing this" +
" wallet can consult reports how many decimal places it" +
" uses, so the amount you are trying to send cannot be" +
" checked against it."
: "Insufficient " +
symbol +
" balance. You have " +
txInfo.tokenBalance +
" " +
symbol +
" but are trying to send " +
txInfo.amount +
" " +
symbol +
".",
);
}
if (codes.includes(CODES.INSUFFICIENT_ETH)) {

View File

@@ -229,6 +229,15 @@ function showFlash(msg, duration = 2000) {
}, duration);
}
// A stored token balance as a number, or null when there is no number in it.
// balances.js writes null for a holding whose scale nothing knows, and this
// keeps that null from becoming a zero one dereference later.
function unknownableAmount(balance) {
if (balance == null) return null;
const n = parseFloat(balance);
return Number.isFinite(n) ? n : null;
}
// One row of the balance list: symbol, quantity, fiat value.
//
// `symbol` is the ERC-20's own symbol() as the block explorer reported it,
@@ -236,9 +245,18 @@ function showFlash(msg, duration = 2000) {
// attacker-chosen length until it has been through displaySymbol. This is
// the row that issue #307 was reported against: every screen that lists a
// holding renders through here.
//
// `amount` is null for a holding whose scale nothing knows
// (https://git.eeqj.de/sneak/AutistMask/issues/349). There is no quantity to
// print for it and no fiat value to derive from one, and printing 0.0000 for
// a real holding is the failure this whole rule exists to prevent, so the row
// says so instead.
function balanceLine(symbol, amount, price, tokenId) {
const qty = amount.toFixed(4);
const usd = price ? formatUsd(amount * price) || "&nbsp;" : "&nbsp;";
const qty = amount === null ? "quantity unknown" : amount.toFixed(4);
const usd =
price && amount !== null
? formatUsd(amount * price) || "&nbsp;"
: "&nbsp;";
// tokenId is a contract address out of the same explorer JSON, and it
// lands inside a quoted attribute.
const tokenAttr = tokenId ? ` data-token="${escapeHtml(tokenId)}"` : "";
@@ -265,7 +283,12 @@ function balanceLinesForAddress(addr, trackedTokens, showZero) {
);
const seen = new Set();
for (const t of addr.tokenBalances || []) {
const bal = parseFloat(t.balance || "0");
// A null balance is a holding of an unstatable amount, not a holding
// of zero, so the show-zero setting has no say over it: hiding it
// would be asserting the zero nobody established. Anything that does
// not parse to a finite number is unknown for the same reason — the
// `|| "0"` this replaced turned both into a confident zero.
const bal = unknownableAmount(t.balance);
if (bal === 0 && !showZero) continue;
html += balanceLine(
t.symbol,
@@ -298,7 +321,12 @@ function addressHoldsFunds(addr) {
if (!addr) return false;
if (parseFloat(addr.balance || "0") > 0) return true;
for (const t of addr.tokenBalances || []) {
if (parseFloat(t.balance || "0") > 0) return true;
// A null balance is a holding whose amount could not be stated —
// balances.js drops a row of zero base units before the scale is
// consulted, so a row that survived with no quantity is holding
// something. Warning about funds must err towards warning.
const bal = unknownableAmount(t.balance);
if (bal === null || bal > 0) return true;
}
return false;
}
@@ -558,6 +586,7 @@ module.exports = {
balanceLine,
balanceLinesForAddress,
addressHoldsFunds,
unknownableAmount,
addressColor,
addressDotHtml,
escapeHtml,

View File

@@ -12,6 +12,7 @@ const {
const { state, currentAddress } = require("../../shared/state");
let ctx;
const { getProvider } = require("../../shared/balances");
const { resolveTokenDecimals } = require("../../shared/approvalAmount");
const { resolveSymbol } = require("../../shared/tokenList");
const { isLowHolderCount } = require("../../shared/holders");
const { isSpoofedSymbol } = require("../../shared/symbolSpoof");
@@ -159,9 +160,14 @@ function updateSendBalance() {
addr.tokenBalances,
state.trackedTokens,
);
const bal = tb ? tb.balance || "0" : "0";
// A null balance is a holding whose scale nothing knows. Saying "0"
// for it would be a claim about the amount; the send itself is
// refused later by transferAmountUnits() for the same missing scale.
const bal = tb ? tb.balance : "0";
$("send-balance").textContent =
"Current balance: " + bal + " " + symbol;
bal == null
? "Current balance: unknown (" + symbol + ")"
: "Current balance: " + bal + " " + symbol;
}
}
@@ -235,8 +241,45 @@ function init(_ctx) {
addr.tokenBalances,
state.trackedTokens,
);
tokenBalance = tb ? tb.balance || "0" : "0";
tokenDecimals = tb ? tb.decimals : null;
// null carried through rather than flattened to "0": the confirm
// screen states an unknown balance as unknown, and
// validateTransfer() treats it as no balance to spend from, which
// is the fail-closed side of an amount nobody can check.
tokenBalance = tb ? (tb.balance ?? null) : "0";
// Resolved the same way balances.js resolved the scale it
// DISPLAYED this token's balance at: bundled list, then the user's
// tracked tokens, then the explorer. The stored
// tokenBalances[].decimals is the explorer's own answer alone, so
// reading it raw carries a null forward for a token the wallet
// does know the scale of — and displayedDecimals() then throws
// inside estimateGas(), which the confirmation screen reports as
// an unestimable fee. Unsendable, over a scale that was never in
// doubt (https://git.eeqj.de/sneak/AutistMask/issues/349).
// Still null when nothing knows: no fallback.
//
// Resolved WITH `wallets`, which balances.js does not pass: that
// adds explorerDecimals()'s cross-address check, so a contract two
// addresses report different scales for answers null rather than
// picking one. That check has to apply here, because this value
// encodes a transfer; balances.js is formatting one explorer row
// at fetch time and cannot consult a state it is in the middle of
// replacing.
tokenDecimals = resolveTokenDecimals(token, {
trackedTokens: state.trackedTokens,
wallets: state.wallets,
});
// The two resolutions can therefore differ, and where they do, the
// stored `balance` is a quantity computed at a scale this screen
// has just declined to stand behind. Stating it would leave
// validateTransfer() checking the amount against a number the
// wallet does not vouch for, and — since the unknown-balance path
// is gated on the balance, not on the scale — would leave the
// fee-estimate failure as the only thing on the confirmation
// screen, which says nothing about decimals. Unknown scale means
// unknown balance. Only a stored quantity is withdrawn: the "0"
// for a token that has no row at all is an absence of holdings,
// which is true at every scale.
if (tb && tokenDecimals === null) tokenBalance = null;
}
ctx.showConfirmTx({

View File

@@ -23,33 +23,14 @@
// disputed is refused rather than guessed at.
// Solidity's decimals() is a uint8, and every source here is ultimately
// reporting that call's result.
const { MAX_DECIMALS } = require("./transferAmount");
// reporting that call's result. toDecimals() is that check, shared with the
// send path rather than copied: the bundled list stores numbers, the
// explorer's copy arrives as a string, and a token the user added by hand
// carries whatever lookupTokenInfo() got back, so the accepted types are
// enumerated rather than coerced.
const { toDecimals } = require("./transferAmount");
const { TOKEN_BY_ADDRESS } = require("./tokenList");
// A decimals value as a number, or null if it is not one. The bundled list
// stores numbers, the explorer's copy arrives as a string, and a token the
// user added by hand can carry whatever lookupTokenInfo() got back, so the
// accepted types are enumerated rather than coerced: Number([]) is 0 and
// Number(true) is 1, so a coercing check would read an empty array as a scale
// of zero and format the amount as whole tokens.
function toDecimals(value) {
let n;
if (typeof value === "number") {
n = value;
} else if (typeof value === "bigint") {
if (value < 0n || value > BigInt(MAX_DECIMALS)) return null;
n = Number(value);
} else if (typeof value === "string") {
if (!/^[0-9]+$/.test(value)) return null;
n = Number(value);
} else {
return null;
}
if (!Number.isInteger(n) || n < 0 || n > MAX_DECIMALS) return null;
return n;
}
// Every decimals the explorer reported for this contract, across all the
// addresses whose balances have been fetched. They describe one contract, so
// they should agree; a set that does not agree is a scale in dispute, and this

View File

@@ -15,6 +15,8 @@ const { deriveAddressFromXpub } = require("./wallet");
const { TOKEN_BY_ADDRESS } = require("./tokenList");
const { LOW_HOLDER_THRESHOLD, parseHoldersCount } = require("./holders");
const { isSpoofedSymbol } = require("./symbolSpoof");
const { toDecimals } = require("./transferAmount");
const { resolveTokenDecimals } = require("./approvalAmount");
// Use a static network to skip auto-detection (which can fail and cause
// "could not coalesce error" on some RPC endpoints like Cloudflare).
@@ -66,10 +68,28 @@ function formatTokenBalance(raw, decimals) {
return parts[0] + "." + dec;
}
// The explorer's reported holding as an exact base-unit integer, or null when
// it reported nothing usable. Base units carry no scale, so this value is
// meaningful before the scale is known — which is what lets a holding of zero
// be recognised as zero without guessing a scale to divide it by.
function rawUnits(value) {
if (typeof value === "bigint") return value >= 0n ? value : null;
if (typeof value === "number") {
return Number.isSafeInteger(value) && value >= 0 ? BigInt(value) : null;
}
if (typeof value !== "string" || !/^[0-9]+$/.test(value)) return null;
return BigInt(value);
}
// Fetch token balances for a single address from Blockscout.
// Returns [{ address, symbol, decimals, balance }].
// Returns [{ address, name, symbol, decimals, balance, holders }].
// Filters out spam: only shows tokens that are in the known token list,
// explicitly tracked by the user, or have >= 1000 holders.
//
// `decimals` and `balance` are each null when the answer is unknown, the same
// way `holders` already is. Absence is never filled in here: this is the
// upstream of every screen that displays a token amount, so a value invented
// at this point is indistinguishable from a real one everywhere below it.
async function fetchTokenBalances(address, blockscoutUrl, trackedTokens) {
try {
const resp = await debugFetch(
@@ -94,11 +114,46 @@ async function fetchTokenBalances(address, blockscoutUrl, trackedTokens) {
// is unchanged.
const type = String(item.token?.type || "").toUpperCase();
if (type !== "ERC-20") continue;
const decimals = parseInt(item.token.decimals || "18", 10);
const bal = formatTokenBalance(item.value || "0", decimals);
if (bal === "0.0") continue;
const tokenAddr = (item.token.address_hash || "").toLowerCase();
// What the explorer reported, or null. NEVER a default: this
// value is written to state and every later reader — the approval
// screen's amount line, the swap lines, the Send screen — takes it
// as the token's resolved scale. A fabricated 18 reads exactly
// like a real 18 at that point, so it does not merely display the
// wrong quantity, it walks straight past the refusal those screens
// already have for a scale nobody knows
// (https://git.eeqj.de/sneak/AutistMask/issues/349).
const decimals = toDecimals(item.token.decimals);
const raw = rawUnits(item.value);
// No usable amount at all is nothing to list, exactly as a
// formatted "0.0" was before. Checked on the base-unit integer so
// it does not depend on knowing the scale: zero base units is zero
// tokens at every scale, and a value the explorer did not report
// as an integer is not a holding.
if (raw === null || raw === 0n) continue;
// The scale this row's balance is DISPLAYED at, which is not the
// same question as what the explorer said. The bundled list and
// the tokens the user tracks both outrank the explorer already
// (resolveTokenDecimals), so a token they know keeps showing its
// real quantity even when the explorer's entry omits decimals.
// Only what neither of them nor the explorer knows is unknown.
// The stored `decimals` above stays the explorer's own answer
// either way: copying another source into it would make
// explorerDecimals()'s disagreement check compare something other
// than explorer values.
const known = resolveTokenDecimals(tokenAddr, { trackedTokens });
const scale = known !== null ? known : decimals;
// null is a holding of an amount that cannot be stated, which is
// not the same as a holding of zero, and must never render as one.
// With a scale, the display filter proper applies: a balance that
// rounds to zero at six places is dust and is not listed. Without
// one there is no such judgement to make, and the row is kept.
const bal = scale === null ? null : formatTokenBalance(raw, scale);
if (bal === "0.0") continue;
// null means the explorer reported no count, which is not the
// same as a count of zero. This gate is not the low-holder
// display filter: it has no user-facing off switch and governs
@@ -127,7 +182,15 @@ async function fetchTokenBalances(address, blockscoutUrl, trackedTokens) {
address: item.token.address_hash,
name: item.token.name || "",
symbol: item.token.symbol || "???",
// null means the explorer reported no usable scale — unknown,
// not 18. Distinguishable from a real 18 at read time is the
// entire point: resolveTokenDecimals() falls through a null to
// its refusal, and takes an 18 as the answer.
decimals: decimals,
// null means nothing anywhere knows the scale, so there is no
// token quantity to state. Not "0.0": a nonzero holding shown
// as zero is the same lie in the balance list that the
// approval screens refuse to tell.
balance: bal,
holders: holders,
});

View File

@@ -85,6 +85,14 @@ function isRecord(value) {
// alternative — refusing the whole record — sends a user whose wallets are
// perfectly readable to an export-or-erase screen over a token list. An entry
// that is a record with a text address is kept verbatim, extra fields and all.
//
// Verbatim is load-bearing for the fields BESIDE the address. A tokenBalances
// entry carries `decimals: null` and `balance: null` when nothing knows the
// token's scale (src/shared/balances.js,
// https://git.eeqj.de/sneak/AutistMask/issues/349), and those nulls are the
// record that the value is unknown. Only `address` decides whether an entry
// survives, so an unknown-scale holding is kept — flooring a null here to some
// default would put the guess back one layer down from where it was removed.
function tokenRefs(value) {
if (!Array.isArray(value)) return [];
return value.filter(

View File

@@ -78,9 +78,18 @@ function getAddressValue(addr) {
let usd = parseFloat(addr.balance || "0") * prices.ETH;
let partial = false;
for (const token of addr.tokenBalances || []) {
const tokenBal = parseFloat(token.balance || "0");
// A null balance is a holding whose scale nothing knows, so it has no
// quantity to price — but it is still a holding, and a total that
// silently omits it would read as complete. That is exactly what
// `partial` is for (https://git.eeqj.de/sneak/AutistMask/issues/349).
if (token.balance == null) {
partial = true;
continue;
}
const tokenBal = parseFloat(token.balance);
// A balance of zero is not a holding: it can neither add to the total
// nor make it incomplete.
// nor make it incomplete. Anything that is not a number at all is not
// a holding this can price either, and is left to the same rule.
if (!(tokenBal > 0)) continue;
if (prices[token.symbol]) {
usd += tokenBal * prices[token.symbol];

View File

@@ -33,10 +33,17 @@
// tests/persistedFieldContract.test.js: one row per persisted field, naming
// the property that field's floor is claimed to have, and PROVING it by
// driving the real code with hostile values — the gate for a field the gate
// refuses, normalizePersisted() for a field it floors, and a boot of the real
// popup entry point for a field whose only defence is that nothing
// dereferences it structurally. A field added to PERSISTED_FIELDS with no row
// fails that suite; so does a row whose claim is false.
// refuses, normalizePersisted() for a field it floors, and, for a field whose
// only defence is that nothing dereferences it structurally, a boot of the
// real popup entry point onto EVERY view the popup can reopen onto.
//
// That last part is the whole point, because this defect class lives on the
// RESTORE path and not on Home: a field that gains a structural dereference
// in any restorable view's render turns that suite red, as does a field that
// gains a floor while its row still claims it has none. What the boot does not
// reach is what no stored record reaches by itself — a view only forward
// navigation opens, and anything behind a click. A field added to
// PERSISTED_FIELDS with no row fails the suite too.
//
// That test exists because this comment did not work. It carried a
// hand-written justification per field, and it shipped a false one in three

View File

@@ -11,6 +11,10 @@ const { log, debugFetch } = require("./log");
const { TOKEN_BY_ADDRESS } = require("./tokenList");
const { parseHoldersCount, isLowHolderCount } = require("./holders");
const { isSpoofedSymbol } = require("./symbolSpoof");
// The uint8 test every scale in this wallet goes through. Shared, not copied:
// a scale is either reported or it is unknown, and "unknown" must mean the
// same thing here as it does on the screens that refuse to format one.
const { toDecimals } = require("./transferAmount");
// The plain 4-decimal rule. The history and balance lists deliberately keep
// truncation without the approval screens' nonzero floor: the transaction
// detail view is the authoritative record and already shows exact precision.
@@ -92,21 +96,37 @@ function parseTx(tx, addrLower) {
function parseTokenTransfer(tt, addrLower) {
const from = tt.from?.hash || "";
const to = tt.to?.hash || "";
const decimals = parseInt(tt.total?.decimals || "18", 10);
// The explorer's own answer, or null. Never a default: a transfer of
// 5000000000 units formatted at a guessed 18 reads as 0.000000005, and
// nothing downstream can tell that from a real 18-decimal transfer of
// that size. `parseInt(x || "18", 10)` also collapsed a genuine scale of
// ZERO into 18 (https://git.eeqj.de/sneak/AutistMask/issues/246).
const decimals = toDecimals(tt.total?.decimals);
const rawVal = tt.total?.value || "0";
const direction =
normalizeAddress(from) === addrLower ? "sent" : "received";
const sym = tt.token?.symbol || "?";
// Without a scale there is no token quantity, so none is stated: the list
// row falls back to the symbol alone and the detail screen to its
// direction label, exactly as the contract-call rows above already do.
// The exact figure is not lost — it is the base-unit line below, which is
// the one number that needs no scale to be true.
const formatted =
decimals === null ? "" : formatTxValue(formatUnits(rawVal, decimals));
const exact = decimals === null ? "" : formatUnits(rawVal, decimals);
return {
hash: tt.transaction_hash,
blockNumber: tt.block_number,
timestamp: Math.floor(new Date(tt.timestamp).getTime() / 1000),
from: from,
to: to,
value: formatTxValue(formatUnits(rawVal, decimals)),
exactValue: formatUnits(rawVal, decimals),
value: formatted,
exactValue: exact,
rawAmount: rawVal,
rawUnit: sym + " base units (10^-" + decimals + ")",
rawUnit:
decimals === null
? sym + " base units (decimals unknown)"
: sym + " base units (10^-" + decimals + ")",
valueGwei: null,
symbol: sym,
direction: direction,

View File

@@ -52,7 +52,7 @@ function mismatchMessage(displayed, onChain) {
);
}
// A decimals value from either source as a number, or null if it is not one.
// A decimals value from any source as a number, or null if it is not one.
// decimals() comes back from ethers as a bigint and the explorer's copy arrives
// as a string, so both of those are accepted alongside a plain number; anything
// fractional, negative, out of uint8 range, or of any other type at all is not.
@@ -60,7 +60,16 @@ function mismatchMessage(displayed, onChain) {
// The types are enumerated rather than coerced because Number() is far too
// willing: Number([]) is 0 and Number(true) is 1, so a coercing check would
// admit an empty array as a scale of zero and encode a whole-token transfer
// against it.
// against it. Absence answers null and never a default, and a real scale of
// ZERO answers 0 — the two are different answers, which is the whole point:
// a falsy-collapsing `value || 18` cannot tell them apart, and neither can a
// reader of what it wrote (https://git.eeqj.de/sneak/AutistMask/issues/246).
//
// Exported because every module that has to decide whether it knows a token's
// scale needs exactly this test, and three separate copies of it is three
// places for the answer to drift: approvalAmount.js resolves the scale the
// approval screens display at, and balances.js decides what the explorer
// actually reported before it is stored.
function toDecimals(value) {
let n;
if (typeof value === "number") {
@@ -110,6 +119,7 @@ module.exports = {
displayedDecimals,
transferAmountUnits,
mismatchMessage,
toDecimals,
MAX_DECIMALS,
UNKNOWN_DISPLAYED_DECIMALS_MESSAGE,
UNREADABLE_CONTRACT_DECIMALS_MESSAGE,

View File

@@ -0,0 +1,282 @@
// What the balance fetcher stores when the block explorer reports no decimals
// for a token, and what the approval screens then display.
//
// https://git.eeqj.de/sneak/AutistMask/issues/349: `fetchTokenBalances()` did
// `parseInt(item.token.decimals || "18", 10)` BEFORE writing the row, so a
// token whose `decimals()` reverts — and which the explorer therefore reports
// no scale for — was stored with a fabricated 18. Nothing downstream could
// tell that from a real 18.
//
// That matters because it is upstream of two refusals that were already built
// and already merged. https://git.eeqj.de/sneak/AutistMask/issues/306 made the
// ERC-20 amount line resolve the real scale or refuse to format, and
// https://git.eeqj.de/sneak/AutistMask/issues/340 did the same for the swap
// lines. Both read this stored value as an authoritative source, so the guess
// walked straight past them: the refusal was intact and simply never fired.
//
// So these tests run a real explorer response through the real fetcher and
// assert on the real approval screens. A test that hand-writes `decimals: null`
// onto state would pass on the broken build, because the fabrication is in the
// writer, not the readers.
jest.mock("../src/shared/log", () => ({
log: {
debugf: () => {},
infof: () => {},
warnf: () => {},
errorf: () => {},
},
debugFetch: jest.fn(),
setRuntimeDebug: () => {},
isDebug: () => false,
}));
global.fetch = jest.fn(() => {
throw new Error("tests must not perform network requests");
});
const { makeStorageStub } = require("./support/storageStub");
global.chrome = { storage: makeStorageStub() };
const { AbiCoder, Interface } = require("ethers");
const { ERC20_ABI } = require("../src/shared/constants");
const { fetchTokenBalances } = require("../src/shared/balances");
const { debugFetch } = require("../src/shared/log");
const { state } = require("../src/shared/state");
const { unknownDecimalsAmount } = require("../src/shared/approvalAmount");
const { decodeCalldata } = require("../src/popup/views/approval");
const { TOKEN_BY_ADDRESS } = require("../src/shared/tokenList");
const HOLDER = "0x" + "a".repeat(40);
const BLOCKSCOUT = "https://blockscout.example/api/v2";
const ROUTER = "0x66a9893cc07d91d95644aedd05d03f95e1dba8af";
const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe";
const SPENDER = "0x1111111111111111111111111111111111111111";
// Outside the bundled list and untracked, so the explorer is the only source
// of a scale for it — which is the case the fabrication was hiding.
const NOVEL = "0xE2E0000000000000000000000000000000000E2e";
// In the bundled list, at 18 decimals, for the other side of a swap.
const WETH = "0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2";
// The holding the explorer reports, in base units. Large enough that it does
// not round to zero even when divided by 10^18, which is what makes it the
// case the laundering actually REACHED: a smaller holding formatted at the
// fabricated 18 comes out "0.0", the balance list drops the row as dust, and
// the approval screens then find no source for the scale and refuse anyway —
// for the wrong reason, and only by luck.
const HOLDING = 5000000000000000000n;
// The amount in the dApp's calldata, which is a separate number from the
// holding. 1,000.00 of a 6-decimal token; formatted at the fabricated 18 it
// reads 0.000000001, and at a real scale of 0 it reads 1000000000.
const THOUSAND_AT_SIX = 1000000000n;
const HALF_WETH = 500000000000000000n;
const erc20Iface = new Interface(ERC20_ABI);
const coder = AbiCoder.defaultAbiCoder();
const routerIface = new Interface([
"function execute(bytes commands, bytes[] inputs, uint256 deadline)",
]);
// One Blockscout token-balances row. `token` is spread last so a test can
// override or blank a field; the base row carries no `decimals` at all, which
// is exactly what a token whose decimals() reverts produces.
function row(token = {}, value = HOLDING) {
return {
value: String(value),
token: {
type: "ERC-20",
address_hash: NOVEL,
symbol: "NOVEL",
name: "Novel Token",
// Well clear of the balance list's own spam floor, so the row is
// admitted on its holder count alone: neither the bundled list nor
// a tracked entry can supply a scale for it.
holders_count: "50000",
...token,
},
};
}
function respondWith(items) {
debugFetch.mockImplementation(async () => ({
ok: true,
status: 200,
statusText: "OK",
json: async () => items,
}));
}
// Fetch and place the result exactly where refreshBalances() places it, so the
// approval screens read what a real refresh would have left on state.
async function fetchOnto(items, trackedTokens = []) {
respondWith(items);
const balances = await fetchTokenBalances(
HOLDER,
BLOCKSCOUT,
trackedTokens,
);
state.trackedTokens = trackedTokens;
state.wallets = [
{
name: "Wallet 1",
addresses: [
{ address: HOLDER, balance: "1.0", tokenBalances: balances },
],
},
];
return balances;
}
// The ERC-20 approval screen's Amount line, and the swap decoder's.
function erc20AmountLine(data, tokenAddress) {
return decodeCalldata(data, tokenAddress).details.find(
(d) => d.label === "Amount",
).value;
}
function swapAmountLine(data) {
return decodeCalldata(data, ROUTER).details.find(
(d) => d.label === "Amount",
).value;
}
function transferData(amount) {
return erc20Iface.encodeFunctionData("transfer", [RECIPIENT, amount]);
}
function approveData(amount) {
return erc20Iface.encodeFunctionData("approve", [SPENDER, amount]);
}
function swapData(tokenIn, amountIn, tokenOut, amountOutMin) {
const input = coder.encode(
["address", "uint256", "uint256", "address[]", "bool"],
[RECIPIENT, amountIn, amountOutMin, [tokenIn, tokenOut], true],
);
return routerIface.encodeFunctionData("execute", [
"0x08",
[input],
9999999999n,
]);
}
beforeEach(() => {
debugFetch.mockReset();
state.trackedTokens = [];
state.wallets = [];
});
describe("what fetchTokenBalances stores for an absent scale", () => {
test("the token is not in the bundled list, so the explorer is the only source", () => {
expect(TOKEN_BY_ADDRESS.has(NOVEL.toLowerCase())).toBe(false);
});
test("an absent decimals is stored as null, not as 18", async () => {
const balances = await fetchOnto([row()]);
expect(balances).toHaveLength(1);
expect(balances[0].decimals).toBeNull();
});
test("an explicit null decimals is stored as null too", async () => {
const balances = await fetchOnto([row({ decimals: null })]);
expect(balances[0].decimals).toBeNull();
});
// The same explorer row twice, differing only in whether it reports a
// scale of 18. Before the fix both stored 18 and no reader could tell
// which one had actually been reported.
test("a real 18 is stored as 18, and so is distinguishable from absent", async () => {
const real = await fetchOnto([row({ decimals: "18" })]);
expect(real[0].decimals).toBe(18);
expect(real[0].balance).toBe("5.0");
const absent = await fetchOnto([row()]);
expect(absent[0].decimals).toBeNull();
expect(real[0].decimals).not.toBe(absent[0].decimals);
});
// The falsy-collapse trap of
// https://git.eeqj.de/sneak/AutistMask/issues/246. `decimals || "18"` reads
// a real scale of zero as absent and then as eighteen, which is eighteen
// orders of magnitude of error in the direction that displays as nothing.
test("a real scale of zero is stored as zero, not collapsed", async () => {
for (const reported of ["0", 0]) {
const balances = await fetchOnto([row({ decimals: reported })]);
expect(balances[0].decimals).toBe(0);
expect(balances[0].balance).toBe("5000000000000000000.0");
}
});
test("no quantity is stated for a holding whose scale is unknown", async () => {
const balances = await fetchOnto([row()]);
// Not "0.0": the holding is real and nonzero, and a zero here is the
// same lie the approval screens refuse to tell.
expect(balances[0].balance).toBeNull();
});
// Zero base units is zero tokens at every scale, so this filter never
// needed a scale in the first place and does not acquire one now.
test("a holding of zero base units is still dropped without a scale", async () => {
expect(await fetchOnto([row({}, 0n)])).toEqual([]);
});
test("the bundled list still supplies a quantity the explorer omitted", async () => {
const balances = await fetchOnto([
row({ address_hash: WETH, symbol: "WETH" }),
]);
// The stored decimals stay the explorer's own answer — absent. Copying
// another source in here would make explorerDecimals()'s disagreement
// check compare something other than explorer values.
expect(balances[0].decimals).toBeNull();
// The displayed quantity still comes out right, because the bundled
// list knows this token's scale and outranks the explorer anyway.
expect(balances[0].balance).toBe("5.0");
});
});
describe("the ERC-20 approval line reaches its refusal", () => {
test("a transfer of a token the explorer gave no scale for is not formatted", async () => {
await fetchOnto([row()]);
const line = erc20AmountLine(transferData(THOUSAND_AT_SIX), NOVEL);
expect(line).toBe(unknownDecimalsAmount(THOUSAND_AT_SIX));
// The defect: a fabricated 18 renders this as 0.000000001, a quantity,
// and a wrong one.
expect(line).not.toMatch(/^0\./);
});
test("an approve of the same token is not formatted either", async () => {
await fetchOnto([row()]);
const line = erc20AmountLine(approveData(THOUSAND_AT_SIX), NOVEL);
expect(line).toBe(unknownDecimalsAmount(THOUSAND_AT_SIX));
expect(line).not.toMatch(/^0\./);
});
test("a scale the explorer did report still formats", async () => {
await fetchOnto([row({ decimals: "6" })]);
expect(erc20AmountLine(transferData(THOUSAND_AT_SIX), NOVEL)).toBe(
"1000.0000",
);
});
});
describe("the swap approval line reaches its refusal", () => {
test("a swap of a token the explorer gave no scale for is not formatted", async () => {
await fetchOnto([row()]);
const line = swapAmountLine(
swapData(NOVEL, THOUSAND_AT_SIX, WETH, HALF_WETH),
);
expect(line).toBe(unknownDecimalsAmount(THOUSAND_AT_SIX));
expect(line).not.toMatch(/^0\./);
});
test("a scale the explorer did report still formats", async () => {
await fetchOnto([row({ decimals: "6" })]);
expect(
swapAmountLine(swapData(NOVEL, THOUSAND_AT_SIX, WETH, HALF_WETH)),
).toBe("1000.0000");
});
});
test("no test in this file performed a network request", () => {
expect(global.fetch).not.toHaveBeenCalled();
});

View File

@@ -47,6 +47,12 @@ const fs = require("fs");
const path = require("path");
const MANIFEST_DIR = path.join(__dirname, "..", "manifest");
const ROOT = path.join(__dirname, "..");
// The sizes both stores and both toolbars ask for.
const EXPECTED_ICON_SIZES = ["16", "32", "48", "128"];
const PNG_SIGNATURE = Buffer.from("89504e470d0a1a0a", "hex");
const EXPECTED_DIRECTIVES = {
"default-src": ["'self'"],
@@ -115,6 +121,51 @@ function assertPolicy(policy) {
}
}
// The declared icons, in both manifests.
//
// Without an "icons" block a browser draws a generic puzzle piece in the
// toolbar for this extension, which is both the first thing the user sees and
// how a real extension is told apart from a look-alike. Declaring one is not
// enough on its own: an entry naming a file that is not in the tree ships a
// reference to nothing, so the referenced bytes are read here and required to
// be a PNG of the size the entry claims. build.js copies these into each
// browser directory, relative to it, and script/lib/package.js then refuses to
// build an archive that does not contain everything the manifest names.
function assertIcons(target) {
const icons = readManifest(target).icons;
expect(Object.keys(icons).sort()).toEqual(EXPECTED_ICON_SIZES.sort());
for (const size of EXPECTED_ICON_SIZES) {
const ref = icons[size];
expect([size, ref]).toEqual([size, `icons/icon${size}.png`]);
const bytes = fs.readFileSync(path.join(ROOT, ref));
expect(bytes.subarray(0, 8)).toEqual(PNG_SIGNATURE);
// IHDR width and height, at fixed offsets right after the signature
// and the chunk header.
expect([ref, bytes.readUInt32BE(16), bytes.readUInt32BE(20)]).toEqual([
ref,
Number(size),
Number(size),
]);
}
}
describe("declared icons", () => {
test("chrome declares real icons at every size", () => {
assertIcons("chrome");
});
test("firefox declares real icons at every size", () => {
assertIcons("firefox");
});
test("both targets declare the same icons", () => {
expect(readManifest("firefox").icons).toEqual(
readManifest("chrome").icons,
);
});
});
describe("shipped Content Security Policy", () => {
// MV3 takes an object and applies extension_pages to the popup and the
// background service worker, which is where libsodium runs.

View File

@@ -90,6 +90,26 @@ describe("archive self-containment", () => {
);
});
// Toolbar icons are the other file the manifest names and no bundler
// emits, so an archive built without them would carry a manifest whose
// "icons" resolve to nothing and a browser would fall back to a generic
// placeholder without saying so.
test("a manifest naming an icon that is not in the archive fails", () => {
const { members, read } = archiveOf({
"manifest.json": JSON.stringify({
...MINIMAL_MANIFEST,
icons: { 16: "icons/icon16.png", 128: "icons/icon128.png" },
}),
"src/popup/index.html": "<html></html>",
"src/popup/index.js": "//",
"src/background/index.js": "//",
"icons/icon16.png": "PNG",
});
expect(() => checkSelfContained("chrome", members, read)).toThrow(
/would not be self-contained.*icons\/icon128\.png/s,
);
});
test("an archive with no manifest.json at its root fails", () => {
const { members, read } = archiveOf({ "src/popup/index.js": "//" });
expect(() => checkSelfContained("chrome", members, read)).toThrow(

View File

@@ -20,12 +20,27 @@
// that no structural dereference of it is reachable from a
// stored record — which cannot be argued, only driven, so the
// proof is a boot of the REAL popup entry point over a stored
// record carrying the hostile value.
// record carrying the hostile value, ONTO EVERY RESTORABLE
// VIEW. Home is not where this class of defect lives.
//
// Every row is driven through the boot regardless of kind, and a LOOSE row
// must additionally prove it is loose: if someone floors the field and leaves
// the row saying LOOSE, the "survives verbatim" assertion fails. A field added
// to PERSISTED_FIELDS with no row fails the first test in the file.
// Every row is driven through a boot regardless of kind, but only a LOOSE row
// (or a row that sets `alsoSweep`) is swept across the restore path: that is
// what declaring LOOSE costs. ENTRIES and SCALAR rows are proven by their
// holds() instead, because a floored value is not hostile by the time a
// renderer sees it. A LOOSE row must additionally prove it is loose: if
// someone floors the field — even partially — and leaves the row saying LOOSE,
// the "survives verbatim" assertion fails. A field added to PERSISTED_FIELDS
// with no row fails the first test in the file.
//
// The sweep is what makes a LOOSE row falsifiable, so read how it is driven
// before trusting it. A row the ROUTER reads (`routes`) gets its own boot per
// view, because a hostile value in it legitimately changes which view renders.
// Every other swept field is corrupted on the SAME boot, one boot per view per
// hostile slot, and that boot has to land on the view it stored — so a field
// that does move the routing cannot hide in the crowd, and a field that is
// dereferenced by any renderer reachable from a stored record turns this file
// red. Booting each of them separately would be about six hundred boots and
// half a minute; this is thirty-three.
//
// The three claims this replaced, all false, all caught here by construction:
// rpcUrl reaching `new JsonRpcProvider()` (a synchronous throw, not a caught
@@ -61,12 +76,22 @@ const isIndexOrNull = (v) => v === null || (Number.isInteger(v) && v >= 0);
const isTextOrNull = (v) => v === null || (isText(v) && v !== "");
const everyEntry = (v, fn) => Array.isArray(v) && v.every(fn);
// A row is SWEPT — driven onto every restorable view rather than only onto
// Home — when its claim is that no restore path dereferences the field. That
// is what LOOSE means. The two index rows opt in with `alsoSweep` although
// they are floored, because the restore path is precisely why they gained a
// floor and the sweep is the regression guard on it.
const swept = (row) => row.kind === KIND.LOOSE || Boolean(row.alsoSweep);
// ------------------------------------------------------------------ the table
//
// `hostile` is values a stored record can carry that nothing in src/ ever
// writes. Each one is driven through the floor AND through a real popup boot,
// so keep the list short and pointed. `floorOnly` is extra values checked
// against the floor alone, which is pure and free.
// writes. Each one is driven through the floor AND through a real popup boot
// and, for a swept row, through one boot per restorable view — so keep the
// list short and pointed. `floorOnly` is extra values checked against the
// floor alone, which is pure and free. `hostileRestore` is extra values driven
// through the restore path only, for a value that means nothing until a
// particular branch's gate has let it past.
const CONTRACT = [
{
@@ -213,6 +238,14 @@ const CONTRACT = [
hostile: ["map", "__proto__", { a: 1 }],
floorOnly: ["length", "constructor", "toString", "0", -1, 1.5, true],
holds: isIndexOrNull,
// SCALAR, and swept anyway: the restore path is precisely why this
// field gained a floor, so the sweep is the regression guard on it.
alsoSweep: true,
routes: true,
// A stale INTEGER index, which reaches the restore path by a different
// route from the prototype members above — falsy or out of range
// rather than truthy — and has to keep being the safe case.
hostileRestore: [{ value: "length" }, { value: 5 }],
},
{
field: "selectedAddress",
@@ -220,10 +253,14 @@ const CONTRACT = [
hostile: ["map", "__proto__", { a: 1 }],
floorOnly: ["length", "constructor", "toString", "0", -1, 1.5, true],
holds: isIndexOrNull,
alsoSweep: true,
routes: true,
hostileRestore: [{ value: 5 }],
},
{
field: "currentView",
kind: KIND.LOOSE,
routes: true,
// Compared, and concatenated into the debug banner's textContent
// (src/popup/views/helpers.js) with no gate in front of it, which
// coerces. Nothing renders FROM it without RESTORABLE_VIEWS.has()
@@ -233,11 +270,86 @@ const CONTRACT = [
{
field: "viewData",
kind: KIND.LOOSE,
routes: true,
// The container is taken verbatim; what makes its ENTRIES safe is the
// per-branch guard in src/popup/viewRouter.js. Driven over every
// restorable view in "a malformed viewData" below, which is the proof
// this row rests on.
hostile: [42, "notarecord", { a: 1 }],
// per-branch guard in src/popup/viewRouter.js. The sweep drives the
// container shapes below onto every restorable view; hostileRestore
// adds the records that PASS a branch's gate and then hand its
// renderer something it dereferences, which is where the entries are
// actually decided.
hostile: [42, "notarecord", { a: 1 }, [1, 2]],
hostileRestore: [
// success-tx passes on `data.hash`, and renderSuccess() then calls
// toAddressHtml(d.to) -> addressTitle() -> address.toLowerCase().
{ value: { hash: "0x1" }, views: ["success-tx"] },
{ value: { hash: "0x1", to: 42 }, views: ["success-tx"] },
{
value: { hash: "0x1", to: ADDRESS, decoded: { details: 7 } },
views: ["success-tx"],
},
{
value: {
hash: "0x1",
to: ADDRESS,
decoded: { details: [{ address: 42 }] },
},
views: ["success-tx"],
},
// error-tx passes on `data.message`, same dereference.
{ value: { message: "boom" }, views: ["error-tx"] },
{ value: { message: "boom", to: 42 }, views: ["error-tx"] },
// transaction passes on `data.tx`.
{ value: { tx: { hash: "0x1" } }, views: ["transaction"] },
{
value: {
tx: {
hash: "0x1",
from: ADDRESS,
to: ADDRESS,
contractAddress: 42,
},
},
views: ["transaction"],
},
// confirm-tx passes on `data.pendingTx`.
{ value: { pendingTx: { amount: "1" } }, views: ["confirm-tx"] },
{
value: {
pendingTx: {
token: 42,
from: ADDRESS,
to: ADDRESS,
amount: "1",
},
},
views: ["confirm-tx"],
},
// wait-tx passes on `pendingWait.hash`; restoreWait() has checked
// the fields below it since it was written, and this is the
// regression guard.
{
value: {
pendingWait: {
hash: "0x1",
txInfo: { to: 42, amount: "1" },
},
},
views: ["wait-tx"],
},
// A record that passes EVERY branch's gate at once, driven onto
// every restorable view: a branch a view does not read must stay
// one it does not read, and each renderer must survive the fields
// another branch left behind.
{
value: {
hash: "0x1",
message: "boom",
tx: { hash: "0x1" },
pendingTx: { amount: "1" },
pendingWait: { hash: "0x1" },
},
},
],
},
{
field: "lastBalanceRefresh",
@@ -354,16 +466,22 @@ describe("the floor each row claims", () => {
if (row.kind === KIND.LOOSE) {
test(`${row.field}: is genuinely unfloored`, () => {
const survived = values.some((value) => {
// EVERY value, not some: a PARTIAL floor is still a floor, and
// a row that keeps saying LOOSE because one hostile value out
// of three still survives is exactly the stale claim this file
// exists to stop.
for (const value of values) {
const out = normalizePersisted(
profileWith(row.field, value),
);
return (
JSON.stringify(out[row.field]) === JSON.stringify(value)
);
});
expect(survived).toBe(true);
expect({
value: value,
survived: JSON.stringify(out[row.field]),
}).toEqual({
value: value,
survived: JSON.stringify(value),
});
}
});
}
}
@@ -385,7 +503,10 @@ async function bootHealth(profile) {
const HEALTHY = { errors: [], blank: false };
describe("a hostile value for one field, through the real popup", () => {
// unversionedValidProfile() stores no currentView, so every boot in here lands
// on Home. That is the cheap half of the proof; the restore path below is the
// half that matters.
describe("a hostile value for one field, booting onto Home", () => {
for (const row of CONTRACT) {
for (const value of row.hostile) {
test(`${row.field} = ${JSON.stringify(value)}`, async () => {
@@ -409,72 +530,49 @@ describe("a row's extra proof against the real reader", () => {
}
});
// ------------------------------------------------- viewData, entry by entry
// ------------------------------------------------ driving the restore path
// The views that read viewData.
const DATA_VIEWS = [
"confirm-tx",
"transaction",
"wait-tx",
"success-tx",
"error-tx",
];
// Everything above lands on Home. Home is not where this class of defect
// lives: all three of the false claims this file replaced were falsified by a
// RESTORE, through the unguarded restoreView() in src/popup/index.js. So a
// swept row's hostile values are driven onto EVERY restorable view, one boot
// each.
//
// This is what makes a LOOSE row falsifiable. A field that gains a structural
// dereference on any restorable view — `state.theme.toLowerCase()` in a view's
// show(), say — turns the row red here, instead of waiting for a reviewer to
// re-derive the claim by hand.
// Each record below PASSES the gate of the branch it names, and then carries a
// value that branch's renderer dereferences. `views` is where it is driven from
// — the whole set for a value that is not a record at all, and otherwise the
// branch it targets, since the cross-view case is covered by EVERY_GATE below.
const HOSTILE_VIEW_DATA = [
{ data: 42, views: DATA_VIEWS },
{ data: "notarecord", views: DATA_VIEWS },
{ data: [1, 2], views: DATA_VIEWS },
// success-tx passes on `data.hash`, and renderSuccess() then calls
// toAddressHtml(d.to) -> addressTitle() -> address.toLowerCase().
{ data: { hash: "0x1" }, views: ["success-tx"] },
{ data: { hash: "0x1", to: 42 }, views: ["success-tx"] },
{
data: { hash: "0x1", to: ADDRESS, decoded: { details: 7 } },
views: ["success-tx"],
// restoreWait() resumes from this, so it has to be a finite number and recent
// enough that the resumed deadline has not already passed — a wait that has
// outlived its deadline resolves on the first poll instead of staying on
// screen. Read once at module load, so every boot in one run shares it.
const BROADCAST_TIME = Date.now();
// A viewData well formed for every restorable branch at once, so the only
// thing a swept boot can fail on is the field the row corrupts. "the base
// profile the sweep corrupts" below proves this really does render each view
// rather than falling back — without that, a sweep could pass by never
// reaching a renderer at all.
const WELL_FORMED_DATA = {
hash: "0x1",
message: "boom",
to: ADDRESS,
decoded: { details: [{ address: TOKEN_ADDRESS }] },
tx: { hash: "0x1", from: ADDRESS, to: ADDRESS, contractAddress: null },
pendingTx: {
token: "ETH",
from: ADDRESS,
to: ADDRESS,
amount: "1",
balance: "2",
},
{
data: {
hash: "0x1",
to: ADDRESS,
decoded: { details: [{ address: 42 }] },
},
views: ["success-tx"],
pendingWait: {
hash: "0x1",
txInfo: { to: ADDRESS, amount: "1" },
broadcastTime: BROADCAST_TIME,
},
// error-tx passes on `data.message`, same dereference.
{ data: { message: "boom" }, views: ["error-tx"] },
{ data: { message: "boom", to: 42 }, views: ["error-tx"] },
// transaction passes on `data.tx`.
{ data: { tx: { hash: "0x1" } }, views: ["transaction"] },
{
data: {
tx: {
hash: "0x1",
from: ADDRESS,
to: ADDRESS,
contractAddress: 42,
},
},
views: ["transaction"],
},
// confirm-tx passes on `data.pendingTx`.
{ data: { pendingTx: { amount: "1" } }, views: ["confirm-tx"] },
{
data: {
pendingTx: { token: 42, from: ADDRESS, to: ADDRESS, amount: "1" },
},
views: ["confirm-tx"],
},
// wait-tx passes on `pendingWait.hash`; restoreWait() has checked the
// fields below it since it was written, and this is the regression guard.
{
data: { pendingWait: { hash: "0x1", txInfo: { to: 42, amount: "1" } } },
views: ["wait-tx"],
},
];
};
function restoringOnto(view, extra) {
return unversionedValidProfile({
@@ -483,88 +581,143 @@ function restoringOnto(view, extra) {
selectedAddress: 0,
selectedToken: TOKEN_ADDRESS,
viewStack: ["main"],
viewData: WELL_FORMED_DATA,
...extra,
});
}
describe("a malformed viewData restoring onto", () => {
for (const { data, views } of HOSTILE_VIEW_DATA) {
for (const view of views) {
test(`${view}: ${JSON.stringify(data)}`, async () => {
await expect(
bootHealth(restoringOnto(view, { viewData: data })),
).resolves.toEqual(HEALTHY);
});
}
}
// Every restorable view, against one record that passes every branch's
// gate at once: a branch a view does not read must stay one it does not
// read, and each renderer must survive the fields another branch left.
const EVERY_GATE = {
hash: "0x1",
message: "boom",
tx: { hash: "0x1" },
pendingTx: { amount: "1" },
pendingWait: { hash: "0x1" },
// A boot that RESTORED is healthy and landed on the view it stored, rather
// than falling back to Home — which a healthy boot also does, and which would
// let a sweep pass by never running the renderer it is aimed at.
async function restoredHealth(profile, view) {
const env = await bootPopup(profile);
return {
errors: env.pageErrors,
restored: env.visibleViews().includes(view),
};
}
const RESTORED = { errors: [], restored: true };
describe("the base profile the sweep corrupts", () => {
for (const view of RESTORABLE_VIEWS) {
test(`${view}: a record passing every branch's gate at once`, async () => {
test(`renders ${view} rather than falling back`, async () => {
await expect(
bootHealth(restoringOnto(view, { viewData: EVERY_GATE })),
).resolves.toEqual(HEALTHY);
restoredHealth(restoringOnto(view), view),
).resolves.toEqual(RESTORED);
});
}
});
// --------------------------------------- selectedWallet / selectedAddress
// A field the ROUTER itself reads — the two it gates on and the two
// hasValidAddress() indexes with. A hostile value in one of these legitimately
// changes which view renders, so each gets its own boot per view and is held
// only to "healthy", not to "restored onto the view it stored".
const routes = (row) => Boolean(row.routes);
// `wallets` is a real Array, so a selectedWallet naming an Array.prototype or
// Object.prototype member is TRUTHY: hasValidAddress()'s `&&` does not
// short-circuit, `.addresses` is undefined, and the index access throws out of
// restoreView(). A stale INTEGER is falsy-or-in-range and safe — the opposite
// way round from how this pair was described.
const HOSTILE_INDEX = [
{ selectedWallet: "map", selectedAddress: 0 },
{ selectedWallet: "length", selectedAddress: 0 },
{ selectedWallet: "__proto__", selectedAddress: 0 },
{ selectedWallet: "constructor", selectedAddress: 0 },
{ selectedWallet: 0, selectedAddress: "map" },
{ selectedWallet: 5, selectedAddress: 0 },
];
// Every routing row × every hostile value × every restorable view. Profiles
// are deduplicated because a hostile `currentView` REPLACES the view being
// restored onto, which would otherwise be the same boot eleven times.
describe("a hostile routing value restoring onto", () => {
for (const row of CONTRACT) {
if (!swept(row) || !routes(row)) continue;
const seen = new Set();
for (const value of row.hostile) {
for (const view of RESTORABLE_VIEWS) {
const profile = restoringOnto(view, { [row.field]: value });
const key = JSON.stringify(profile);
if (seen.has(key)) continue;
seen.add(key);
test(`${view}: ${row.field} = ${JSON.stringify(
value,
)}`, async () => {
await expect(bootHealth(profile)).resolves.toEqual(HEALTHY);
});
}
}
}
});
const INDEX_VIEWS = [
"address",
"address-token",
"receive",
"transaction",
"confirm-tx",
];
// Every OTHER swept field, corrupted at once, one boot per view per hostile
// slot: twelve fields on one boot rather than twelve boots. A field is only in
// here because it is not one the router reads — and that is ASSERTED, not
// argued, because the boot has to land on `view`. A field that does move the
// routing turns this red and has to declare `routes` and take the individual
// sweep above.
//
// Nothing is masked by combining: a throw fails the boot whichever field threw,
// and the only other way a dereference could go unseen is the renderer not
// running at all, which is exactly what `restored` forbids. When it does go
// red, the same view is re-booted one field at a time so the failure names the
// fields rather than leaving a reader to bisect twelve of them.
const UNROUTED = CONTRACT.filter((row) => swept(row) && !routes(row));
const HOSTILE_SLOTS = Math.max(...UNROUTED.map((row) => row.hostile.length));
describe("a malformed wallet or address index restoring onto", () => {
const WELL_FORMED_DATA = {
tx: { hash: "0x1", from: ADDRESS, to: ADDRESS },
pendingTx: {
token: "ETH",
from: ADDRESS,
to: ADDRESS,
amount: "1",
balance: "2",
},
};
function unroutedValues(slot) {
const fields = {};
for (const row of UNROUTED) {
fields[row.field] = row.hostile[slot % row.hostile.length];
}
return fields;
}
for (const view of INDEX_VIEWS) {
for (const indices of HOSTILE_INDEX) {
test(`${view}: ${JSON.stringify(indices)}`, async () => {
await expect(
bootHealth(
describe("every field the router does not read, corrupted at once, onto", () => {
for (const view of RESTORABLE_VIEWS) {
for (let slot = 0; slot < HOSTILE_SLOTS; slot++) {
test(`${view}: hostile value ${slot + 1} in all ${
UNROUTED.length
} of them`, async () => {
const fields = unroutedValues(slot);
const together = await restoredHealth(
restoringOnto(view, fields),
view,
);
if (together.errors.length === 0 && together.restored) {
expect(together).toEqual(RESTORED);
return;
}
const named = [];
for (const row of UNROUTED) {
const one = await restoredHealth(
restoringOnto(view, {
...indices,
viewData: WELL_FORMED_DATA,
[row.field]: fields[row.field],
}),
),
).resolves.toEqual(HEALTHY);
view,
);
if (one.errors.length === 0 && one.restored) continue;
named.push(
`${row.field}=${JSON.stringify(fields[row.field])}: ` +
(one.errors.join("; ") || `fell off ${view}`),
);
}
expect({ view: view, fields: named }).toEqual({
view: view,
fields: [],
});
});
}
}
});
// The values that only mean something on the restore path: a viewData that
// PASSES a branch's gate and then hands its renderer something dereferenced,
// and the index values whose route through hasValidAddress() differs from the
// row's own hostile set.
describe("a restore-only hostile value onto", () => {
for (const row of CONTRACT) {
for (const entry of row.hostileRestore || []) {
for (const view of entry.views || RESTORABLE_VIEWS) {
test(`${view}: ${row.field} = ${JSON.stringify(
entry.value,
)}`, async () => {
await expect(
bootHealth(
restoringOnto(view, { [row.field]: entry.value }),
),
).resolves.toEqual(HEALTHY);
});
}
}
}
});

View File

@@ -120,6 +120,20 @@ function makeElement(id, className) {
el.clicked += 1;
},
};
// src/ reaches parentElement only to hide or unhide the wrapper a field
// sits in (txStatus.js renderSuccess(), transactionDetail.js render()).
// The stub is flat — it is built from the ids in the markup, not from its
// tree — so each element gets a wrapper of its own, made on demand so this
// does not recurse. It is never registered by id, so nothing can mistake
// it for a view. Without it, success-tx and transaction throw on the first
// line that touches a wrapper and cannot be booted onto at all.
let parent = null;
Object.defineProperty(el, "parentElement", {
get() {
if (!parent) parent = makeElement(id + "-parent", "");
return parent;
},
});
return el;
}

View File

@@ -0,0 +1,185 @@
// What the screens that READ a stored token balance do with a holding whose
// scale nothing knows.
//
// https://git.eeqj.de/sneak/AutistMask/issues/349 stopped `fetchTokenBalances()`
// fabricating a scale of 18, so a row it cannot state a quantity for is now
// stored with `balance: null`. Every reader of that field therefore has two
// distinct inputs where it used to have one, and the property that has to hold
// at each of them is the same one this codebase keeps losing:
//
// null (unknown) and 0 (genuinely zero) must produce DIFFERENT output.
//
// Losing it is what https://git.eeqj.de/sneak/AutistMask/issues/246,
// https://git.eeqj.de/sneak/AutistMask/issues/306,
// https://git.eeqj.de/sneak/AutistMask/issues/322,
// https://git.eeqj.de/sneak/AutistMask/issues/359 and
// https://git.eeqj.de/sneak/AutistMask/issues/364 each were. So every case
// below asserts the pair, not just that the null branch does something
// reasonable: an assertion on the null alone still passes on a build that
// renders both as zero, which is precisely the build being guarded against.
//
// The writer half — that the fetcher stores null rather than 18 — is in
// tests/fabricatedDecimals.test.js, and the Send and confirmation screens are
// in tests/unknownScaleSend.test.js.
"use strict";
// helpers.js reaches for both at module scope through the modules it pulls in.
globalThis.chrome = {
storage: {
local: {
get: () => Promise.resolve({}),
set: () => Promise.resolve(),
},
},
runtime: { sendMessage: () => {} },
};
globalThis.document = {
getElementById: () => null,
createElement: () => ({ style: {}, classList: { toggle() {} } }),
body: { prepend: () => {} },
addEventListener: () => {},
};
const {
balanceLine,
balanceLinesForAddress,
addressHoldsFunds,
} = require("../src/popup/views/helpers");
const {
prices,
clearPrices,
getAddressValue,
} = require("../src/shared/prices");
const { state } = require("../src/shared/state");
const NOVEL = "0x1111111111111111111111111111111111111111";
// One stored tokenBalances row. `balance: null` is what balances.js writes for
// a holding whose scale nothing knows; "0.0" is a quantity that was actually
// established and is zero.
function holding(balance) {
return {
address: NOVEL,
symbol: "NOVEL",
decimals: balance === null ? null : 18,
balance,
holders: 50000,
};
}
function address(balance) {
return {
address: "0x" + "a".repeat(40),
balance: "0",
tokenBalances: [holding(balance)],
};
}
// The quantity cell of a rendered row, which is the second of the two spans
// inside the fixed-width span.
function quantities(html) {
return [...html.matchAll(/<span>([^<]*)<\/span>/g)].map((m) => m[1]);
}
beforeEach(() => {
clearPrices();
state.wallets = [];
state.trackedTokens = [];
state.activeAddress = null;
});
afterEach(() => {
clearPrices();
});
describe("balanceLine", () => {
test("an unknown quantity and a zero one render differently", () => {
const unknown = balanceLine("NOVEL", null, null, NOVEL);
const zero = balanceLine("NOVEL", 0, null, NOVEL);
expect(unknown).not.toBe(zero);
expect(quantities(unknown)).toEqual(["NOVEL", "quantity unknown"]);
expect(quantities(zero)).toEqual(["NOVEL", "0.0000"]);
});
test("an unknown quantity produces no fiat figure, a zero one does", () => {
prices.NOVEL = 3;
const unknown = balanceLine("NOVEL", null, 3, NOVEL);
const zero = balanceLine("NOVEL", 0, 3, NOVEL);
// A price times an unknown quantity is not $0.00: that is the same
// claim of "nothing here" the quantity cell just refused to make.
expect(unknown).toContain(
'<span class="text-right text-muted flex-1">&nbsp;</span>',
);
expect(zero).toContain(
'<span class="text-right text-muted flex-1">$0.00</span>',
);
});
});
describe("balanceLinesForAddress", () => {
// The show-zero setting is a statement about zeroes. An unknown quantity
// is not one, so hiding the row would assert the zero nobody established
// and the holding would vanish from the list entirely.
test("hiding zero balances hides the zero row and keeps the unknown one", () => {
const unknown = balanceLinesForAddress(address(null), [], false);
const zero = balanceLinesForAddress(address("0.0"), [], false);
expect(unknown).not.toBe(zero);
expect(unknown).toContain("quantity unknown");
expect(unknown).toContain("NOVEL");
expect(zero).not.toContain("NOVEL");
});
test("showing zero balances still tells the two apart", () => {
const unknown = balanceLinesForAddress(address(null), [], true);
const zero = balanceLinesForAddress(address("0.0"), [], true);
expect(unknown).not.toBe(zero);
expect(quantities(unknown)).toEqual([
"ETH",
"0.0000",
"NOVEL",
"quantity unknown",
]);
expect(quantities(zero)).toEqual(["ETH", "0.0000", "NOVEL", "0.0000"]);
});
});
describe("addressHoldsFunds", () => {
// Read by deleteAddress.js to decide whether removing the address is
// warned about. balances.js drops a row of zero base units before any
// scale is consulted, so a row that survived with no quantity is holding
// something, and the warning must err towards warning.
test("an unknown balance holds funds, a zero balance does not", () => {
expect(addressHoldsFunds(address(null))).toBe(true);
expect(addressHoldsFunds(address("0.0"))).toBe(false);
});
});
describe("getAddressValue", () => {
// `usd` is the value of what could be priced and `partial` says it is a
// floor rather than the total. An unpriceable holding is exactly what
// `partial` exists for; a holding of zero can neither add to the total nor
// make it incomplete.
test("an unknown balance makes the total partial, a zero balance does not", () => {
prices.ETH = 2000;
prices.NOVEL = 3;
const unknown = getAddressValue(address(null));
const zero = getAddressValue(address("0.0"));
expect(unknown).not.toEqual(zero);
expect(unknown).toEqual({ usd: 0, partial: true });
expect(zero).toEqual({ usd: 0, partial: false });
});
test("an unknown balance is not priced as zero of the token", () => {
prices.ETH = 2000;
prices.NOVEL = 3;
// The same row with a real quantity of 10 is worth $30. Neither that
// figure nor a confident $0.00 may be stated for the unknown one.
expect(getAddressValue(address("10.0"))).toEqual({
usd: 30,
partial: false,
});
expect(getAddressValue(address(null)).usd).toBe(0);
expect(getAddressValue(address(null)).partial).toBe(true);
});
});

View File

@@ -0,0 +1,455 @@
// The Send and confirmation screens for a token whose explorer row carries no
// decimals.
//
// https://git.eeqj.de/sneak/AutistMask/issues/349 made `fetchTokenBalances()`
// store the explorer's own answer — `null` when it reported none — while the
// scale a balance is DISPLAYED at is resolved separately: bundled list, then
// the user's tracked tokens, then the explorer. The two are different
// questions, and `tokenBalances[].decimals` only answers the second one.
//
// A reader that takes the stored field for the display scale therefore gets
// `null` for a token the wallet does know the scale of. On the Send path that
// null reaches `displayedDecimals()` inside `estimateGas()`, which throws, is
// caught as an unavailable fee, and disables Send behind "The network fee could
// not be estimated" — untrue, unactionable, and for a bundled token like WETH
// or DAI whose scale was never in doubt. So the Send screen resolves the scale
// the same way the balance list did, and only carries a null forward when that
// resolution genuinely answers null.
//
// Driven through the real `fetchTokenBalances()`, the real Send review handler
// and the real confirmation screen: a test that hand-wrote `decimals: null`
// onto state would not show which of the two questions each screen is asking.
//
// The reader sites that are pure display are in tests/unknownScaleDisplay.test.js,
// and what the fetcher stores is in tests/fabricatedDecimals.test.js.
"use strict";
jest.mock("../src/shared/log", () => ({
log: {
debugf: () => {},
infof: () => {},
warnf: () => {},
errorf: () => {},
},
debugFetch: jest.fn(),
setRuntimeDebug: () => {},
isDebug: () => false,
}));
// Everything the confirmation screen would reach the network for. The gas
// estimate is the point: with a usable scale it must succeed, so that a failure
// in these tests is a failure of the scale and not of the stub.
const mockProvider = {
getFeeData: async () => ({
maxFeePerGas: 2000000000n,
gasPrice: 1000000000n,
}),
estimateGas: async () => 21000n,
getCode: async () => "0x",
getTransactionCount: async () => 1,
getBalance: async () => 0n,
};
jest.mock("../src/shared/balances", () => {
const actual = jest.requireActual("../src/shared/balances");
return { ...actual, getProvider: () => mockProvider };
});
// The confirmation screen's best-effort Etherscan label lookup is the one
// thing here that reaches for fetch(). It is stubbed to fail, which is the
// path it already takes offline; the assertion at the bottom of this file
// pins that it is the ONLY fetch these screens make.
global.fetch = jest.fn(() => {
throw new Error("tests must not perform network requests");
});
const { makeStorageStub } = require("./support/storageStub");
global.chrome = { storage: makeStorageStub(), runtime: { sendMessage() {} } };
// A stub DOM. Every id in index.html that these two views touch resolves to a
// fresh recording element; nothing here depends on layout, only on what the
// views write into the elements and which handlers they register.
const elements = new Map();
function makeEl(id) {
const handlers = new Map();
return {
id,
textContent: "",
innerHTML: "",
value: "",
disabled: false,
onclick: null,
style: {},
dataset: {},
classList: {
add() {},
remove() {},
toggle() {},
contains: () => false,
},
handlers,
addEventListener(name, fn) {
handlers.set(name, fn);
},
appendChild(child) {
return child;
},
querySelectorAll: () => [],
querySelector: () => null,
remove() {},
focus() {},
};
}
global.document = {
getElementById(id) {
if (!elements.has(id)) elements.set(id, makeEl(id));
return elements.get(id);
},
createElement: (tag) => makeEl(tag),
body: { prepend() {}, appendChild() {} },
addEventListener() {},
};
global.navigator = { clipboard: { writeText() {} } };
const { parseUnits } = require("ethers");
const { fetchTokenBalances } = require("../src/shared/balances");
const { debugFetch } = require("../src/shared/log");
const { state } = require("../src/shared/state");
const {
displayedDecimals,
transferAmountUnits,
} = require("../src/shared/transferAmount");
const send = require("../src/popup/views/send");
const confirmTx = require("../src/popup/views/confirmTx");
const { TOKEN_BY_ADDRESS } = require("../src/shared/tokenList");
const HOLDER = "0x" + "a".repeat(40);
const SECOND_HOLDER = "0x" + "b".repeat(40);
const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe";
const BLOCKSCOUT = "https://blockscout.example/api/v2";
// Bundled, 18 decimals. The wallet knows this token's scale without asking
// anyone, which is what makes an unsendable WETH a regression rather than a
// refusal.
const WETH = "0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2";
// Neither bundled nor tracked, so the explorer is the only possible source and
// an omission there really is an unknown scale.
const NOVEL = "0xE2E0000000000000000000000000000000000E2e";
const FIVE_WETH = 5000000000000000000n;
function row(token = {}, value = FIVE_WETH) {
return {
value: String(value),
token: {
type: "ERC-20",
address_hash: WETH,
symbol: "WETH",
name: "Wrapped Ether",
holders_count: "50000",
...token,
},
};
}
// Fetch the explorer's rows through the real fetcher and put them exactly where
// refreshBalances() puts them.
async function fetchOnto(items) {
debugFetch.mockImplementation(async () => ({
ok: true,
status: 200,
statusText: "OK",
json: async () => items,
}));
const balances = await fetchTokenBalances(HOLDER, BLOCKSCOUT, []);
state.wallets = [
{
name: "Wallet 1",
addresses: [
{ address: HOLDER, balance: "1.0", tokenBalances: balances },
],
},
];
state.selectedWallet = 0;
state.selectedAddress = 0;
return balances;
}
// The same, for two addresses of one wallet holding the same contract. Sending
// is from the first. Two addresses is what it takes to reach
// explorerDecimals()'s disagreement check, which is only reachable across rows.
async function fetchOntoBoth(itemsA, itemsB) {
debugFetch.mockImplementation(async () => ({
ok: true,
status: 200,
statusText: "OK",
json: async () => itemsA,
}));
const a = await fetchTokenBalances(HOLDER, BLOCKSCOUT, []);
debugFetch.mockImplementation(async () => ({
ok: true,
status: 200,
statusText: "OK",
json: async () => itemsB,
}));
const b = await fetchTokenBalances(SECOND_HOLDER, BLOCKSCOUT, []);
state.wallets = [
{
name: "Wallet 1",
addresses: [
{ address: HOLDER, balance: "1.0", tokenBalances: a },
{ address: SECOND_HOLDER, balance: "1.0", tokenBalances: b },
],
},
];
state.selectedWallet = 0;
state.selectedAddress = 0;
return { a, b };
}
function el(id) {
return global.document.getElementById(id);
}
// Press Review on the Send screen and return the txInfo it hands the
// confirmation screen.
async function reviewSend(tokenAddress, amount) {
let handed = null;
send.init({ showConfirmTx: (info) => (handed = info) });
state.selectedToken = tokenAddress;
el("send-token").value = tokenAddress;
el("send-to").value = RECIPIENT;
el("send-amount").value = amount;
await el("btn-send-review").handlers.get("click")();
return handed;
}
// show() kicks off the gas estimate without awaiting it; this lets it settle.
async function settle() {
for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0));
}
function text(id) {
return el(id).textContent;
}
function errors() {
return el("confirm-errors").innerHTML;
}
function sendDisabled() {
return el("btn-confirm-send").disabled;
}
beforeEach(() => {
elements.clear();
debugFetch.mockReset();
state.wallets = [];
state.trackedTokens = [];
state.selectedToken = null;
state.fraudContracts = [];
state.hideLowHolderTokens = false;
state.currentView = null;
});
describe("the Send screen resolves the scale rather than reading the stored one", () => {
test("the bundled list knows WETH, and the explorer row does not report a scale", async () => {
expect(TOKEN_BY_ADDRESS.get(WETH.toLowerCase()).decimals).toBe(18);
const balances = await fetchOnto([row()]);
// Stored: the explorer's own answer, which is nothing. Reading THIS is
// what carried a null into the fee estimate.
expect(balances[0].decimals).toBeNull();
// Displayed: the bundled scale, so the quantity on screen is real.
expect(balances[0].balance).toBe("5.0");
});
test("the review hands the confirmation screen the resolved scale, not the stored null", async () => {
const balances = await fetchOnto([row()]);
const txInfo = await reviewSend(WETH, "1.5");
expect(txInfo.tokenDecimals).toBe(18);
expect(txInfo.tokenDecimals).not.toBe(balances[0].decimals);
expect(txInfo.tokenBalance).toBe("5.0");
});
test("that scale estimates a fee and leaves Send enabled", async () => {
await fetchOnto([row()]);
const txInfo = await reviewSend(WETH, "1.5");
confirmTx.show(txInfo);
await settle();
// The regression: displayedDecimals(null) threw in estimateGas(), the
// catch reported the fee as unknown, and Send stayed disabled behind a
// message about the network fee that no retry could clear.
expect(text("confirm-fee-amount")).not.toBe("Unable to estimate");
expect(text("confirm-fee-amount")).toContain("ETH");
expect(errors()).toBe("");
expect(sendDisabled()).toBe(false);
});
test("and the transfer encodes at the scale that was displayed", async () => {
await fetchOnto([row()]);
const txInfo = await reviewSend(WETH, "1.5");
// The two calls confirmTx makes with this field: the gas estimate's
// scale, and the encode, which compares it against the contract's own
// decimals() before parsing.
expect(displayedDecimals(txInfo.tokenDecimals)).toBe(18);
expect(
transferAmountUnits(txInfo.amount, txInfo.tokenDecimals, 18n),
).toBe(parseUnits("1.5", 18));
});
test("a token nothing knows the scale of is still refused, and says why", async () => {
await fetchOnto([
row({ address_hash: NOVEL, symbol: "NOVEL", name: "Novel Token" }),
]);
const txInfo = await reviewSend(NOVEL, "1.5");
// No fallback was introduced: resolution answers null here, and the
// null is what goes forward.
expect(txInfo.tokenDecimals).toBeNull();
expect(txInfo.tokenBalance).toBeNull();
confirmTx.show(txInfo);
await settle();
expect(text("confirm-balance")).toBe("unknown (NOVEL)");
expect(errors()).toContain("This token&#39;s balance is unknown");
expect(sendDisabled()).toBe(true);
});
});
// balances.js resolves the display scale WITHOUT `wallets`, so its explorer leg
// is the row it is formatting. send.js resolves WITH `wallets`, so its explorer
// leg is explorerDecimals(), which answers null when two addresses report
// different scales for one contract — the check that must apply before a scale
// encodes a transfer. The two therefore disagree exactly here, and a stored
// balance formatted at a scale the Send screen just refused is not a balance it
// may state: it would leave validateTransfer() satisfied, the unknown-balance
// sentence unfired, and the fee-estimate failure as the only thing on screen.
describe("a scale the explorer's own rows disagree about", () => {
// 5000000 units at the "6" address A reports, 5e18 at the "18" address B
// reports: both format to "5.0", so the disagreement is in the scale alone
// and not in the quantity.
function novel(decimals, value) {
return row(
{
address_hash: NOVEL,
symbol: "NOVEL",
name: "Novel Token",
decimals,
},
value,
);
}
test("is stored per row, because storage holds the explorer's own answer", async () => {
const { a, b } = await fetchOntoBoth(
[novel("6", 5000000n)],
[novel("18", FIVE_WETH)],
);
expect(a[0].decimals).toBe(6);
expect(a[0].balance).toBe("5.0");
expect(b[0].decimals).toBe(18);
});
test("resolves to null on the Send screen, and takes the balance with it", async () => {
await fetchOntoBoth([novel("6", 5000000n)], [novel("18", FIVE_WETH)]);
const txInfo = await reviewSend(NOVEL, "1.5");
expect(txInfo.tokenDecimals).toBeNull();
// The regression this closes: null scale alongside a non-null balance.
expect(txInfo.tokenBalance).toBeNull();
});
test("so the user is told the balance is unknown, not only that the fee failed", async () => {
await fetchOntoBoth([novel("6", 5000000n)], [novel("18", FIVE_WETH)]);
const txInfo = await reviewSend(NOVEL, "1.5");
confirmTx.show(txInfo);
await settle();
// Before the fix: "5.0 NOVEL", an empty confirm-errors, and
// confirm-fee-unknown-error — "the network fee could not be
// estimated... please go back and try again" — as the only explanation
// for a screen that can never proceed.
expect(errors()).not.toBe("");
expect(errors()).toContain("This token&#39;s balance is unknown");
expect(text("confirm-balance")).toBe("unknown (NOVEL)");
expect(sendDisabled()).toBe(true);
// The fee line still reports the estimate as unavailable, because it
// genuinely is — displayedDecimals() refuses the same missing scale.
// What changed is that it is no longer the ONLY thing on the screen,
// and no longer the only offered explanation. This is exactly how the
// token nothing knows the scale of already behaved.
expect(text("confirm-fee-amount")).toBe("Unable to estimate");
expect(el("confirm-fee-unknown-error").style.visibility).toBe(
"visible",
);
});
test("while agreeing rows leave the scale usable", async () => {
await fetchOntoBoth([novel("6", 5000000n)], [novel("6", 5000000n)]);
const txInfo = await reviewSend(NOVEL, "1.5");
expect(txInfo.tokenDecimals).toBe(6);
expect(txInfo.tokenBalance).toBe("5.0");
confirmTx.show(txInfo);
await settle();
expect(text("confirm-balance")).toBe("5.0 NOVEL");
expect(errors()).toBe("");
expect(sendDisabled()).toBe(false);
});
});
describe("the confirmation screen tells an unknown balance from a zero one", () => {
function txInfo(tokenBalance) {
return {
from: HOLDER,
to: RECIPIENT,
ensName: null,
amount: "1.5",
token: NOVEL,
balance: "1.0",
tokenSymbol: "NOVEL",
tokenBalance,
tokenDecimals: tokenBalance === null ? null : 18,
};
}
async function render(tokenBalance) {
state.wallets = [
{
name: "Wallet 1",
addresses: [
{ address: HOLDER, balance: "1.0", tokenBalances: [] },
],
},
];
state.selectedWallet = 0;
state.selectedAddress = 0;
confirmTx.show(txInfo(tokenBalance));
await settle();
return { balance: text("confirm-balance"), errors: errors() };
}
test("the balance line states unknown rather than a quantity of zero", async () => {
const unknown = await render(null);
const zero = await render("0.0");
expect(unknown.balance).not.toBe(zero.balance);
expect(unknown.balance).toBe("unknown (NOVEL)");
expect(zero.balance).toBe("0.0 NOVEL");
});
// Both hit INSUFFICIENT_TOKEN — an unknown balance is treated as nothing to
// spend from, which is the fail-closed side — but "you have 0.0" is a claim
// about the holding, and this one has no established quantity to claim.
test("the insufficient-balance message names the reason, not a figure", async () => {
const unknown = await render(null);
const zero = await render("0.0");
expect(unknown.errors).not.toBe(zero.errors);
expect(unknown.errors).toContain("This token&#39;s balance is unknown");
expect(unknown.errors).not.toContain("You have");
expect(zero.errors).toContain("You have 0.0 NOVEL");
expect(zero.errors).not.toContain("balance is unknown");
});
});
test("the only network these screens reached for is the Etherscan label lookup", () => {
for (const [url] of global.fetch.mock.calls) {
expect(String(url)).toMatch(/^https:\/\/etherscan\.io\/address\//);
}
});