diff --git a/README.md b/README.md index 3cbf3d7..0485ec1 100644 --- a/README.md +++ b/README.md @@ -902,6 +902,18 @@ 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. + #### Partial USD totals Prices are fetched for the top 25 tokens only, so an address can hold assets the diff --git a/TODO.md b/TODO.md index 6c3a269..25aae90 100644 --- a/TODO.md +++ b/TODO.md @@ -107,6 +107,33 @@ 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. The only `18`s left in `src/` are native + ETH's real scale in `src/shared/uniswap.js` and the fixed-point comparison + scale in `src/shared/txValidation.js`. + - 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 diff --git a/src/popup/views/addressToken.js b/src/popup/views/addressToken.js index a73e651..1431be4 100644 --- a/src/popup/views/addressToken.js +++ b/src/popup/views/addressToken.js @@ -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 || " "; diff --git a/src/popup/views/confirmTx.js b/src/popup/views/confirmTx.js index 7395f26..aa4d97b 100644 --- a/src/popup/views/confirmTx.js +++ b/src/popup/views/confirmTx.js @@ -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)) { diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index 87bc357..5adabb7 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -204,9 +204,27 @@ 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. +// 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; +} + function balanceLine(symbol, amount, price, tokenId) { - const qty = amount.toFixed(4); - const usd = price ? formatUsd(amount * price) || " " : " "; + const qty = amount === null ? "quantity unknown" : amount.toFixed(4); + const usd = + price && amount !== null + ? formatUsd(amount * price) || " " + : " "; // 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)}"` : ""; @@ -233,7 +251,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, @@ -266,7 +289,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; } @@ -525,6 +553,7 @@ module.exports = { balanceLine, balanceLinesForAddress, addressHoldsFunds, + unknownableAmount, addressColor, addressDotHtml, escapeHtml, diff --git a/src/popup/views/send.js b/src/popup/views/send.js index d88ac05..b314459 100644 --- a/src/popup/views/send.js +++ b/src/popup/views/send.js @@ -159,9 +159,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,7 +240,11 @@ function init(_ctx) { addr.tokenBalances, state.trackedTokens, ); - tokenBalance = tb ? tb.balance || "0" : "0"; + // 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 ? tb.balance : null) : "0"; tokenDecimals = tb ? tb.decimals : null; } diff --git a/src/shared/approvalAmount.js b/src/shared/approvalAmount.js index e9c3540..0c1ed16 100644 --- a/src/shared/approvalAmount.js +++ b/src/shared/approvalAmount.js @@ -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 diff --git a/src/shared/balances.js b/src/shared/balances.js index a74f8f9..0e9ad5c 100644 --- a/src/shared/balances.js +++ b/src/shared/balances.js @@ -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, }); diff --git a/src/shared/persistedState.js b/src/shared/persistedState.js index 414cbb1..17ff82b 100644 --- a/src/shared/persistedState.js +++ b/src/shared/persistedState.js @@ -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( diff --git a/src/shared/prices.js b/src/shared/prices.js index fb60399..02a1618 100644 --- a/src/shared/prices.js +++ b/src/shared/prices.js @@ -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]; diff --git a/src/shared/stateSchema.js b/src/shared/stateSchema.js index fb12cbc..fd7b884 100644 --- a/src/shared/stateSchema.js +++ b/src/shared/stateSchema.js @@ -36,7 +36,13 @@ // indexed, assigned into, or .toLowerCase()'d — where a truthy value of // the wrong type throws on the first read. The entries matter as much as // the container: [1, 2] IS a list, and `t.address` is one level below the -// Array.isArray(). +// Array.isArray(). What is checked on an ENTRY is the field the check +// exists for and no more — for trackedTokens and tokenBalances that is +// `address` alone; the rest of an entry is taken verbatim. So an entry's +// `decimals` and `balance` may be null, which is how balances.js records +// that nothing knows the token's scale +// (https://git.eeqj.de/sneak/AutistMask/issues/349), and every reader +// handles that null rather than being defended from it here. // Container shape only: allowedSites, deniedSites. A falsy value or a list // becomes {}; anything else is taken as stored and the entries are not // checked. diff --git a/src/shared/transactions.js b/src/shared/transactions.js index 033543f..3ae38da 100644 --- a/src/shared/transactions.js +++ b/src/shared/transactions.js @@ -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, diff --git a/src/shared/transferAmount.js b/src/shared/transferAmount.js index 2779f16..1ddcf70 100644 --- a/src/shared/transferAmount.js +++ b/src/shared/transferAmount.js @@ -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, diff --git a/tests/fabricatedDecimals.test.js b/tests/fabricatedDecimals.test.js new file mode 100644 index 0000000..8392522 --- /dev/null +++ b/tests/fabricatedDecimals.test.js @@ -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(); +});