diff --git a/TODO.md b/TODO.md index fd5e7dd..5a7cafc 100644 --- a/TODO.md +++ b/TODO.md @@ -44,6 +44,20 @@ but the review is broader than any of them. # Completed Steps +- 2026-08-20: The dApp approval screen no longer shows a token transfer it + cannot scale as `0.0000` + ([#306](https://git.eeqj.de/sneak/AutistMask/issues/306)). `decodeCalldata` + read decimals from the 512-entry bundled token list alone and fell back to 18, + so every token outside it — most of them, including anything the user added by + contract address — was displayed at the wrong scale: a `transfer` of 5,000 + units of a 6-decimal token read as `0.0000`, and a user who reads zero + confirms the drain. The new `src/shared/approvalAmount.js` resolves the scale + from the bundled list, then `state.trackedTokens`, then the decimals the block + explorer already reported in `addr.tokenBalances`, and refuses one the + explorer's own entries disagree about. Where no source knows it, the amount + line is not formatted at all: it shows the base-unit integer and states that + the scale is unknown, for `approve` as well as `transfer`. An unbounded + allowance still reads `Unlimited`, which needs no scale. - 2026-08-20: A web page can no longer switch the wallet's chain, and switching no longer destroys the user's endpoints ([#308](https://git.eeqj.de/sneak/AutistMask/issues/308)). diff --git a/src/popup/views/approval.js b/src/popup/views/approval.js index 167a9a5..f353bdd 100644 --- a/src/popup/views/approval.js +++ b/src/popup/views/approval.js @@ -21,6 +21,10 @@ const { const { getPrice, formatUsd } = require("../../shared/prices"); const { ERC20_ABI } = require("../../shared/constants"); const { TOKEN_BY_ADDRESS } = require("../../shared/tokenList"); +const { + resolveTokenDecimals, + unknownDecimalsAmount, +} = require("../../shared/approvalAmount"); const { decryptWithPassword } = require("../../shared/vault"); const { getSignerForAddress } = require("../../shared/wallet"); const { walletDefect } = require("../../shared/walletDefects"); @@ -43,6 +47,23 @@ function formatTxValue(val) { return parts[0] + "." + dec; } +// The amount line for a decoded ERC-20 call. With a known scale it is the +// token quantity; with `decimals` null it is the base-unit integer with the +// unknown scale stated, because formatting it with an assumed scale is what +// showed a 5,000-token transfer as `0.0000`. `raw` is what the status screens +// carry, `display` is what the approval screen shows. +function tokenAmountText(rawAmount, decimals, symbol) { + if (decimals === null) { + const unknown = unknownDecimalsAmount(rawAmount); + return { raw: unknown, display: unknown }; + } + const formatted = formatTxValue(formatUnits(rawAmount, decimals)); + return { + raw: formatted, + display: formatted + (symbol ? " " + symbol : ""), + }; +} + function tokenLabel(address) { const t = TOKEN_BY_ADDRESS.get(address.toLowerCase()); return t ? t.symbol : null; @@ -59,7 +80,15 @@ function decodeCalldata(data, toAddress) { if (parsed) { const token = TOKEN_BY_ADDRESS.get(toAddress.toLowerCase()); const tokenSymbol = token ? token.symbol : null; - const tokenDecimals = token ? token.decimals : 18; + // null when no source knows this token's scale. It is not + // defaulted to 18: an amount formatted with a guessed scale is + // the wrong number, and for a token with fewer decimals than the + // guess it is the wrong number in the direction that reads as + // zero. See tokenAmountText(). + const tokenDecimals = resolveTokenDecimals(toAddress, { + trackedTokens: state.trackedTokens, + wallets: state.wallets, + }); const contractLabel = tokenSymbol ? tokenSymbol + " (" + toAddress + ")" : toAddress; @@ -71,12 +100,11 @@ function decodeCalldata(data, toAddress) { "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", ); const isUnlimited = rawAmount === maxUint; - const amountRaw = isUnlimited - ? "Unlimited" - : formatTxValue(formatUnits(rawAmount, tokenDecimals)); - const amountStr = isUnlimited - ? "Unlimited" - : amountRaw + (tokenSymbol ? " " + tokenSymbol : ""); + // An unbounded allowance needs no scale to describe, so it is + // still named rather than refused. + const amount = isUnlimited + ? { raw: "Unlimited", display: "Unlimited" } + : tokenAmountText(rawAmount, tokenDecimals, tokenSymbol); return { name: "Token Approval", @@ -97,8 +125,8 @@ function decodeCalldata(data, toAddress) { }, { label: "Amount", - value: amountStr, - rawValue: amountRaw, + value: amount.display, + rawValue: amount.raw, }, ], }; @@ -107,11 +135,11 @@ function decodeCalldata(data, toAddress) { if (parsed.name === "transfer") { const to = parsed.args[0]; const rawAmount = parsed.args[1]; - const amountRaw = formatTxValue( - formatUnits(rawAmount, tokenDecimals), + const amount = tokenAmountText( + rawAmount, + tokenDecimals, + tokenSymbol, ); - const amountStr = - amountRaw + (tokenSymbol ? " " + tokenSymbol : ""); return { name: "Token Transfer", @@ -128,8 +156,8 @@ function decodeCalldata(data, toAddress) { { label: "Recipient", value: to, address: to }, { label: "Amount", - value: amountStr, - rawValue: amountRaw, + value: amount.display, + rawValue: amount.raw, }, ], }; diff --git a/src/shared/approvalAmount.js b/src/shared/approvalAmount.js new file mode 100644 index 0000000..066b558 --- /dev/null +++ b/src/shared/approvalAmount.js @@ -0,0 +1,103 @@ +// The scale an ERC-20 amount in a dApp's calldata is displayed with, and what +// to display when there is no such scale. +// +// The approval screen decodes `transfer` and `approve` calldata into a +// quantity the user confirms against. That quantity is a base-unit integer, +// and turning it into a number a person can read needs the token's decimals. +// Assuming a scale is how a drain gets confirmed: a `transfer` of 5000000000 +// units of a 6-decimal token is 5,000 tokens, but formatted with the ERC-20 +// default of 18 it reads `0.0000`, and a user who reads zero signs. +// +// So a scale is either found or the amount is not formatted. Decimals are +// looked for in the bundled token list, then in the tokens the user tracks, +// then in what the block explorer reported for the contract; where none of +// them answers, unknownDecimalsAmount() renders the base-unit integer with the +// unknown scale stated, and no formatUnits() call is reached at all. +// +// This is the display counterpart to transferAmount.js, which takes the same +// stance on the wallet's own send path: an amount whose scale is unknown or +// 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"); +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 +// screen has no way to tell which member is the true one. +function explorerDecimals(lower, wallets) { + let found = null; + for (const wallet of wallets || []) { + for (const addr of wallet.addresses || []) { + for (const tb of addr.tokenBalances || []) { + if ((tb.address || "").toLowerCase() !== lower) continue; + const d = toDecimals(tb.decimals); + if (d === null) continue; + if (found !== null && found !== d) return null; + found = d; + } + } + } + return found; +} + +// The decimals to render a token amount with, or null when nothing knows. +// `sources` is { trackedTokens, wallets }, both shaped as they are on `state`. +function resolveTokenDecimals(tokenAddress, sources) { + const lower = (tokenAddress || "").toLowerCase(); + if (!lower) return null; + + const bundled = TOKEN_BY_ADDRESS.get(lower); + if (bundled) { + const d = toDecimals(bundled.decimals); + if (d !== null) return d; + } + + const tracked = ((sources && sources.trackedTokens) || []).find( + (t) => (t.address || "").toLowerCase() === lower, + ); + if (tracked) { + const d = toDecimals(tracked.decimals); + if (d !== null) return d; + } + + return explorerDecimals(lower, sources && sources.wallets); +} + +// What the amount line reads when the scale is unknown. The base units are +// exact and the caveat is part of the same string, so the number on the screen +// cannot be mistaken for a token quantity, and it can never read as zero for a +// transfer that is not zero. +function unknownDecimalsAmount(rawAmount) { + return String(rawAmount) + " base units (decimals unknown)"; +} + +module.exports = { + resolveTokenDecimals, + unknownDecimalsAmount, +}; diff --git a/tests/approvalAmount.test.js b/tests/approvalAmount.test.js new file mode 100644 index 0000000..9cb35df --- /dev/null +++ b/tests/approvalAmount.test.js @@ -0,0 +1,208 @@ +// The quantity the dApp approval screen shows for a decoded ERC-20 call. +// +// The screen's amount line is the only place a user sees how much a page is +// asking for, and it is decoded from calldata, which carries base units and +// no scale. Issue #306: decodeCalldata read decimals from the bundled token +// list alone and fell back to 18, so a `transfer` of 5000000000 units of a +// 6-decimal token — 5,000 tokens — was displayed as `0.0000` and confirmed. +// +// What is asserted here is that the scale is found wherever the wallet +// already has it, and that where it is nowhere at all no formatted number is +// produced: the amount line has to say base units and say the scale is +// unknown, because a wrong quantity that reads as zero is worse than an +// unwieldy correct one. + +globalThis.chrome = { + storage: { local: { get: async () => ({}), set: async () => {} } }, +}; + +const { Interface } = require("ethers"); +const { ERC20_ABI } = require("../src/shared/constants"); +const { state } = require("../src/shared/state"); +const { + resolveTokenDecimals, + unknownDecimalsAmount, +} = require("../src/shared/approvalAmount"); +const { decodeCalldata } = require("../src/popup/views/approval"); + +const iface = new Interface(ERC20_ABI); + +// Outside the bundled list, as the great majority of ERC-20s are. +const NOVEL_TOKEN = "0xE2E0000000000000000000000000000000000E2e"; +// In the bundled list, at 6 decimals. +const USDC = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"; +const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe"; +const SPENDER = "0x1111111111111111111111111111111111111111"; + +// 5,000 units of a 6-decimal token, the amount from the issue. +const FIVE_THOUSAND_AT_SIX = 5000000000n; +const MAX_UINT256 = (1n << 256n) - 1n; + +function transferData(amount) { + return iface.encodeFunctionData("transfer", [RECIPIENT, amount]); +} + +function approveData(amount) { + return iface.encodeFunctionData("approve", [SPENDER, amount]); +} + +// The Amount line as the approval screen renders it. +function amountLine(data, tokenAddress) { + const decoded = decodeCalldata(data, tokenAddress); + const detail = decoded.details.find((d) => d.label === "Amount"); + return detail.value; +} + +// A wallet holding `token` with the decimals the block explorer reported, +// shaped as balances.js writes it onto state. +function walletsHolding(token, decimals) { + return [ + { + name: "Wallet 1", + addresses: [ + { + address: "0x" + "a".repeat(40), + balance: "1.0", + tokenBalances: [ + { + address: token, + symbol: "NOVEL", + decimals, + balance: "5000.0", + }, + ], + }, + ], + }, + ]; +} + +beforeEach(() => { + state.trackedTokens = []; + state.wallets = []; +}); + +describe("resolveTokenDecimals", () => { + test("prefers the bundled list", () => { + state.trackedTokens = [{ address: USDC, symbol: "USDC", decimals: 2 }]; + expect(resolveTokenDecimals(USDC, state)).toBe(6); + }); + + test("reads a token the user tracks", () => { + state.trackedTokens = [ + { + address: NOVEL_TOKEN.toLowerCase(), + symbol: "NOVEL", + decimals: 6, + }, + ]; + expect(resolveTokenDecimals(NOVEL_TOKEN, state)).toBe(6); + }); + + test("reads the decimals the explorer reported", () => { + // Blockscout's copy arrives as a string. + state.wallets = walletsHolding(NOVEL_TOKEN, "6"); + expect(resolveTokenDecimals(NOVEL_TOKEN, state)).toBe(6); + }); + + test("falls past a tracked entry whose decimals are unusable", () => { + state.trackedTokens = [ + { address: NOVEL_TOKEN, symbol: "NOVEL", decimals: NaN }, + ]; + state.wallets = walletsHolding(NOVEL_TOKEN, 6); + expect(resolveTokenDecimals(NOVEL_TOKEN, state)).toBe(6); + }); + + test("refuses a scale the explorer's own entries disagree about", () => { + const wallets = walletsHolding(NOVEL_TOKEN, 6); + wallets[0].addresses.push({ + address: "0x" + "b".repeat(40), + balance: "0.0", + tokenBalances: [ + { address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 18 }, + ], + }); + state.wallets = wallets; + expect(resolveTokenDecimals(NOVEL_TOKEN, state)).toBeNull(); + }); + + test("rejects values that are not a uint8", () => { + for (const decimals of [-1, 256, 1.5, true, [], {}, null, "6.0", ""]) { + state.trackedTokens = [{ address: NOVEL_TOKEN, decimals }]; + expect(resolveTokenDecimals(NOVEL_TOKEN, state)).toBeNull(); + } + }); + + test("is null when nothing knows the token", () => { + expect(resolveTokenDecimals(NOVEL_TOKEN, state)).toBeNull(); + }); +}); + +describe("decodeCalldata amount", () => { + test("transfer of a tracked 6-decimal token shows the true quantity", () => { + state.trackedTokens = [ + { address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 6 }, + ]; + expect( + amountLine(transferData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN), + ).toBe("5000.0000"); + }); + + test("transfer priced off the explorer's decimals shows the true quantity", () => { + state.wallets = walletsHolding(NOVEL_TOKEN, "6"); + expect( + amountLine(transferData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN), + ).toBe("5000.0000"); + }); + + test("transfer of an unknown-decimals token shows base units, not a number", () => { + const line = amountLine( + transferData(FIVE_THOUSAND_AT_SIX), + NOVEL_TOKEN, + ); + expect(line).toBe("5000000000 base units (decimals unknown)"); + expect(line).toBe(unknownDecimalsAmount(FIVE_THOUSAND_AT_SIX)); + // The defect: any rendering that reads as a token quantity, and above + // all one that reads as zero. + expect(line).not.toMatch(/0\.0000/); + }); + + test("approve of a tracked 6-decimal token shows the true quantity", () => { + state.trackedTokens = [ + { address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 6 }, + ]; + expect(amountLine(approveData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN)).toBe( + "5000.0000", + ); + }); + + test("approve of an unknown-decimals token shows base units, not a number", () => { + const line = amountLine(approveData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN); + expect(line).toBe("5000000000 base units (decimals unknown)"); + expect(line).not.toMatch(/0\.0000/); + }); + + test("an unbounded allowance is still named, with or without a scale", () => { + expect(amountLine(approveData(MAX_UINT256), NOVEL_TOKEN)).toBe( + "Unlimited", + ); + expect(amountLine(approveData(MAX_UINT256), USDC)).toBe("Unlimited"); + }); + + test("a bundled token keeps its symbol and its scale", () => { + expect(amountLine(transferData(FIVE_THOUSAND_AT_SIX), USDC)).toBe( + "5000.0000 USDC", + ); + }); + + test("the amount carried to the status screens is the same string", () => { + const decoded = decodeCalldata( + transferData(FIVE_THOUSAND_AT_SIX), + NOVEL_TOKEN, + ); + const detail = decoded.details.find((d) => d.label === "Amount"); + expect(detail.rawValue).toBe( + "5000000000 base units (decimals unknown)", + ); + }); +});