diff --git a/TODO.md b/TODO.md index b9b333e..258a1a1 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,16 @@ but the review is broader than any of them. # Completed Steps +- 2026-09-21: The dApp approval and transaction-status screens resolve a token's + symbol from the bundled list, then the tokens the user tracks, then the block + explorer's report — the same sources and precedence the amount line already + used for the token's scale + ([#323](https://git.eeqj.de/sneak/AutistMask/issues/323), folding in + [#354](https://git.eeqj.de/sneak/AutistMask/issues/354)). A token the user + added by hand, or holds a balance of, is now named rather than labelled + `Unknown token`, and a non-bundled ERC-20 is no longer carried onto the wait + screen as `ETH`. A tracked or explorer-reported name stays subject to the + spoof rule, so resolving a symbol is not a new way to wear a known ticker. - 2026-08-30: An address no longer wraps, or is shortened to fit, in any of the common views ([#380](https://git.eeqj.de/sneak/AutistMask/issues/380)). The wallet list was the reported case: the address shared one row with the diff --git a/src/popup/views/approval.js b/src/popup/views/approval.js index 2e468c9..db02d61 100644 --- a/src/popup/views/approval.js +++ b/src/popup/views/approval.js @@ -20,9 +20,9 @@ const { } = require("ethers"); const { getPrice, formatUsd } = require("../../shared/prices"); const { ERC20_ABI } = require("../../shared/constants"); -const { TOKEN_BY_ADDRESS } = require("../../shared/tokenList"); const { resolveTokenDecimals, + resolveTokenSymbol, unknownDecimalsAmount, } = require("../../shared/approvalAmount"); // Four decimals, with the nonzero floor these screens hold: every amount this @@ -63,9 +63,15 @@ function tokenAmountText(rawAmount, decimals, symbol) { }; } +// The symbol shown for a token line, resolved from the bundled list, the +// tokens the user tracks, and the explorer's report — the same chain the +// amount line's scale comes from. Null when no source names one, so the token +// lines keep saying `Unknown token` for a token nothing knows. function tokenLabel(address) { - const t = TOKEN_BY_ADDRESS.get(address.toLowerCase()); - return t ? t.symbol : null; + return resolveTokenSymbol(address, { + trackedTokens: state.trackedTokens, + wallets: state.wallets, + }); } // Try to decode calldata using known ABIs. @@ -85,8 +91,7 @@ function decodeCalldata(data, toAddress) { try { const parsed = erc20Iface.parseTransaction({ data }); if (parsed) { - const token = TOKEN_BY_ADDRESS.get(toAddress.toLowerCase()); - const tokenSymbol = token ? token.symbol : null; + const tokenSymbol = resolveTokenSymbol(toAddress, decimalsSources); // 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 @@ -242,8 +247,11 @@ function showTxApproval(details) { const approvedTx = details.approvedTx; const toAddr = approvedTx.to; - const token = toAddr ? TOKEN_BY_ADDRESS.get(toAddr.toLowerCase()) : null; const ethValue = formatEther(approvedTx.value || "0"); + const sources = { + trackedTokens: state.trackedTokens, + wallets: state.wallets, + }; // Build txInfo for status screens pendingTxDetails = { @@ -251,14 +259,17 @@ function showTxApproval(details) { to: toAddr || "", amount: formatTxValue(ethValue), token: "ETH", - tokenSymbol: token ? token.symbol : null, + tokenSymbol: null, }; // If this is an ERC-20 call, try to extract the real recipient and amount const decoded = decodeCalldata(approvedTx.data, toAddr || ""); if (decoded && decoded.details) { - let decodedTokenAddr = null; - let decodedTokenSymbol = null; + // The asset the status summary is counted in: an ERC-20 call's Token + // contract, or a swap's input token. Its symbol is resolved from the + // same sources as the approval screen, so a non-bundled token the + // wallet knows is not carried onto the wait and success screens as ETH. + let assetAddr = null; for (const d of decoded.details) { if (d.label === "Recipient" && d.address) { pendingTxDetails.to = d.address; @@ -266,20 +277,20 @@ function showTxApproval(details) { if (d.label === "Amount") { pendingTxDetails.amount = d.rawValue || d.value; } - if (d.label === "Token In" && d.isToken && d.address) { - const t = TOKEN_BY_ADDRESS.get(d.address.toLowerCase()); - if (t) { - decodedTokenAddr = d.address; - decodedTokenSymbol = t.symbol; - } + if ( + (d.label === "Token" || d.label === "Token In") && + d.isToken && + d.address + ) { + assetAddr = d.address; } } - if (token) { - pendingTxDetails.token = toAddr; - pendingTxDetails.tokenSymbol = token.symbol; - } else if (decodedTokenAddr) { - pendingTxDetails.token = decodedTokenAddr; - pendingTxDetails.tokenSymbol = decodedTokenSymbol; + if (assetAddr) { + pendingTxDetails.token = assetAddr; + pendingTxDetails.tokenSymbol = resolveTokenSymbol( + assetAddr, + sources, + ); } } diff --git a/src/popup/views/txStatus.js b/src/popup/views/txStatus.js index a79404c..7284f37 100644 --- a/src/popup/views/txStatus.js +++ b/src/popup/views/txStatus.js @@ -13,7 +13,7 @@ const { displaySymbol, clearViewStack, } = require("./helpers"); -const { TOKEN_BY_ADDRESS } = require("../../shared/tokenList"); +const { resolveTokenSymbol } = require("../../shared/approvalAmount"); const { state } = require("../../shared/state"); const { getProvider } = require("../../shared/balances"); const { log } = require("../../shared/log"); @@ -232,9 +232,15 @@ function showSuccess(txInfo, txHash, blockNumber) { ctx.doRefreshAndRender(); } +// The symbol shown for a decoded token line, resolved from the bundled list, +// the tokens the user tracks, and the explorer's report — the same chain the +// approval screen uses. Null when no source names one, so the line keeps +// saying `Unknown token`. function tokenLabel(address) { - const t = TOKEN_BY_ADDRESS.get(address.toLowerCase()); - return t ? t.symbol : null; + return resolveTokenSymbol(address, { + trackedTokens: state.trackedTokens, + wallets: state.wallets, + }); } function decodedDetailsHtml(decoded) { diff --git a/src/shared/approvalAmount.js b/src/shared/approvalAmount.js index 0c1ed16..53ab436 100644 --- a/src/shared/approvalAmount.js +++ b/src/shared/approvalAmount.js @@ -30,6 +30,7 @@ // enumerated rather than coerced. const { toDecimals } = require("./transferAmount"); const { TOKEN_BY_ADDRESS } = require("./tokenList"); +const { isSpoofedSymbol } = require("./symbolSpoof"); // Every decimals the explorer reported for this contract, across all the // addresses whose balances have been fetched. They describe one contract, so @@ -74,6 +75,59 @@ function resolveTokenDecimals(tokenAddress, sources) { return explorerDecimals(lower, sources && sources.wallets); } +// Every symbol the explorer reported for this contract, across the addresses +// whose balances have been fetched. The counterpart to explorerDecimals(): one +// contract, so the reports should agree, and a set that does not agree is a +// name this screen has no way to choose between. +function explorerSymbol(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; + if (!tb.symbol) continue; + if (found !== null && found !== tb.symbol) return null; + found = tb.symbol; + } + } + } + return found; +} + +// The symbol to label a token with, or null when no source the wallet trusts +// names one — in which case the screen keeps saying `Unknown token` rather than +// guessing. The bundled list, then the tokens the user tracks, then what the +// explorer reported: the same sources and the same precedence +// resolveTokenDecimals() uses, so a token's name and its scale are drawn from +// the same place and the two can no longer disagree about which sources they +// trust. `sources` is { trackedTokens, wallets }, shaped as on `state`. +// +// A tracked or explorer-reported symbol is attacker-influenced text, so it is +// held to the spoof rule (symbolSpoof.js): a candidate that wears a bundled or +// native ticker from a contract not entitled to it is refused and the next +// source tried, so resolving a symbol never becomes a new way to claim a known +// ticker. The bundled list is the wallet's own data and is trusted as it is. +function resolveTokenSymbol(tokenAddress, sources) { + const lower = (tokenAddress || "").toLowerCase(); + if (!lower) return null; + + const bundled = TOKEN_BY_ADDRESS.get(lower); + if (bundled && bundled.symbol) return bundled.symbol; + + const tracked = ((sources && sources.trackedTokens) || []).find( + (t) => (t.address || "").toLowerCase() === lower, + ); + const candidates = []; + if (tracked && tracked.symbol) candidates.push(tracked.symbol); + const reported = explorerSymbol(lower, sources && sources.wallets); + if (reported) candidates.push(reported); + + for (const symbol of candidates) { + if (!isSpoofedSymbol(symbol, tokenAddress)) return symbol; + } + return null; +} + // 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 @@ -84,5 +138,6 @@ function unknownDecimalsAmount(rawAmount) { module.exports = { resolveTokenDecimals, + resolveTokenSymbol, unknownDecimalsAmount, }; diff --git a/src/shared/uniswap.js b/src/shared/uniswap.js index 81e3b2e..c147728 100644 --- a/src/shared/uniswap.js +++ b/src/shared/uniswap.js @@ -2,10 +2,10 @@ // swap details. Designed to be extended with other DEX decoders later. const { Interface, AbiCoder, getBytes, formatUnits } = require("ethers"); -const { TOKEN_BY_ADDRESS } = require("./tokenList"); const { truncateAmountNeverZero } = require("./amountDisplay"); const { resolveTokenDecimals, + resolveTokenSymbol, unknownDecimalsAmount, } = require("./approvalAmount"); @@ -123,9 +123,8 @@ function tokenInfo(address, sources) { if (address === "0x0000000000000000000000000000000000000000") { return { symbol: "ETH", decimals: 18, address: null }; } - const t = TOKEN_BY_ADDRESS.get(address.toLowerCase()); return { - symbol: t ? t.symbol : null, + symbol: resolveTokenSymbol(address, sources), decimals: resolveTokenDecimals(address, sources), address, }; diff --git a/tests/approvalAmount.test.js b/tests/approvalAmount.test.js index 9cb35df..86be059 100644 --- a/tests/approvalAmount.test.js +++ b/tests/approvalAmount.test.js @@ -143,16 +143,18 @@ describe("decodeCalldata amount", () => { state.trackedTokens = [ { address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 6 }, ]; + // The tracked entry supplies both: the scale (5000.0000) and, since + // issue #323, the symbol that the scale is counted in. expect( amountLine(transferData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN), - ).toBe("5000.0000"); + ).toBe("5000.0000 NOVEL"); }); 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"); + ).toBe("5000.0000 NOVEL"); }); test("transfer of an unknown-decimals token shows base units, not a number", () => { @@ -172,7 +174,7 @@ describe("decodeCalldata amount", () => { { address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 6 }, ]; expect(amountLine(approveData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN)).toBe( - "5000.0000", + "5000.0000 NOVEL", ); }); diff --git a/tests/fabricatedDecimals.test.js b/tests/fabricatedDecimals.test.js index 8392522..24bc2db 100644 --- a/tests/fabricatedDecimals.test.js +++ b/tests/fabricatedDecimals.test.js @@ -253,8 +253,9 @@ describe("the ERC-20 approval line reaches its refusal", () => { test("a scale the explorer did report still formats", async () => { await fetchOnto([row({ decimals: "6" })]); + // The same explorer entry now also names the token (issue #323). expect(erc20AmountLine(transferData(THOUSAND_AT_SIX), NOVEL)).toBe( - "1000.0000", + "1000.0000 NOVEL", ); }); }); @@ -273,7 +274,7 @@ describe("the swap approval line reaches its refusal", () => { await fetchOnto([row({ decimals: "6" })]); expect( swapAmountLine(swapData(NOVEL, THOUSAND_AT_SIX, WETH, HALF_WETH)), - ).toBe("1000.0000"); + ).toBe("1000.0000 NOVEL"); }); }); diff --git a/tests/resolveTokenSymbol.test.js b/tests/resolveTokenSymbol.test.js new file mode 100644 index 0000000..d093e87 --- /dev/null +++ b/tests/resolveTokenSymbol.test.js @@ -0,0 +1,156 @@ +// The symbol the dApp approval and status screens label a token with. +// +// Issue #323: the approval screen labelled anything outside the bundled list +// `Unknown token`, even a token the user tracks or holds a balance of, while +// the amount line already read that token's *scale* from those same sources +// (issue #306). The name and the scale disagreed about which sources they +// trust. resolveTokenSymbol() closes that gap: it draws the symbol from the +// bundled list, then the tracked tokens, then the explorer's report — the +// precedence resolveTokenDecimals() uses — and returns null, not a guess, +// when nothing names it, so the screens keep saying `Unknown token`. +// +// A tracked or explorer-reported symbol is attacker-influenced text, so it +// stays subject to the spoof rule (src/shared/symbolSpoof.js): resolving a +// symbol must not become a new way for a stray contract to wear a bundled or +// native ticker. + +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 { resolveTokenSymbol } = 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: USDC at 6 decimals, DAI at 18. +const USDC = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"; +const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe"; +const FIVE_THOUSAND_AT_SIX = 5000000000n; + +function transferData(amount) { + return iface.encodeFunctionData("transfer", [RECIPIENT, amount]); +} + +// A wallet whose block-explorer balance for `token` reports `symbol`, shaped +// as balances.js writes it onto state. +function walletsReporting(token, symbol) { + return [ + { + name: "Wallet 1", + addresses: [ + { + address: "0x" + "a".repeat(40), + balance: "1.0", + tokenBalances: [ + { + address: token, + symbol, + decimals: 6, + balance: "5000.0", + }, + ], + }, + ], + }, + ]; +} + +beforeEach(() => { + state.trackedTokens = []; + state.wallets = []; +}); + +describe("resolveTokenSymbol", () => { + test("reads the bundled list", () => { + expect(resolveTokenSymbol(USDC, state)).toBe("USDC"); + }); + + test("prefers the bundled list over a tracked entry", () => { + state.trackedTokens = [{ address: USDC, symbol: "NOTUSDC" }]; + expect(resolveTokenSymbol(USDC, state)).toBe("USDC"); + }); + + test("reads a token the user tracks", () => { + state.trackedTokens = [{ address: NOVEL_TOKEN, symbol: "NOVEL" }]; + expect(resolveTokenSymbol(NOVEL_TOKEN, state)).toBe("NOVEL"); + }); + + test("reads the symbol the explorer reported", () => { + state.wallets = walletsReporting(NOVEL_TOKEN, "NOVEL"); + expect(resolveTokenSymbol(NOVEL_TOKEN, state)).toBe("NOVEL"); + }); + + test("is null when no source names the token", () => { + expect(resolveTokenSymbol(NOVEL_TOKEN, state)).toBeNull(); + }); + + test("refuses a name the explorer's own entries disagree about", () => { + const wallets = walletsReporting(NOVEL_TOKEN, "NOVEL"); + wallets[0].addresses.push({ + address: "0x" + "b".repeat(40), + balance: "0.0", + tokenBalances: [{ address: NOVEL_TOKEN, symbol: "OTHER" }], + }); + state.wallets = wallets; + expect(resolveTokenSymbol(NOVEL_TOKEN, state)).toBeNull(); + }); + + test("rejects a tracked entry claiming a bundled ticker it is not", () => { + // NOVEL_TOKEN is not the real USDC contract, so it may not wear USDC. + state.trackedTokens = [{ address: NOVEL_TOKEN, symbol: "USDC" }]; + expect(resolveTokenSymbol(NOVEL_TOKEN, state)).toBeNull(); + }); + + test("rejects an explorer entry claiming the native ETH ticker", () => { + state.wallets = walletsReporting(NOVEL_TOKEN, "ETH"); + expect(resolveTokenSymbol(NOVEL_TOKEN, state)).toBeNull(); + }); +}); + +describe("decodeCalldata symbol", () => { + test("a tracked token is named, not called Unknown", () => { + state.trackedTokens = [ + { address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 6 }, + ]; + const decoded = decodeCalldata( + transferData(FIVE_THOUSAND_AT_SIX), + NOVEL_TOKEN, + ); + expect(decoded.description).toBe("Transfer NOVEL"); + const amount = decoded.details.find((d) => d.label === "Amount"); + expect(amount.value).toBe("5000.0000 NOVEL"); + }); + + test("a token nothing knows keeps a symbol-less label", () => { + const decoded = decodeCalldata( + transferData(FIVE_THOUSAND_AT_SIX), + NOVEL_TOKEN, + ); + expect(decoded.description).toBe("Transfer ERC-20 token"); + const token = decoded.details.find((d) => d.label === "Token"); + // The Token line carries the address and is flagged for the screen's + // symbol lookup, which resolves to nothing here — so `Unknown token`. + expect(token.isToken).toBe(true); + expect(token.address).toBe(NOVEL_TOKEN); + expect(resolveTokenSymbol(token.address, state)).toBeNull(); + }); + + test("a tracked token spoofing a bundled ticker is not named by it", () => { + state.trackedTokens = [ + { address: NOVEL_TOKEN, symbol: "USDC", decimals: 6 }, + ]; + const decoded = decodeCalldata( + transferData(FIVE_THOUSAND_AT_SIX), + NOVEL_TOKEN, + ); + expect(decoded.description).toBe("Transfer ERC-20 token"); + const amount = decoded.details.find((d) => d.label === "Amount"); + expect(amount.value).not.toMatch(/USDC/); + }); +}); diff --git a/tests/uniswapTokenOut.test.js b/tests/uniswapTokenOut.test.js index 9979d11..e7c38c8 100644 --- a/tests/uniswapTokenOut.test.js +++ b/tests/uniswapTokenOut.test.js @@ -78,15 +78,20 @@ describe("a swap to a token absent from the bundled list", () => { ); }); - test("names the address when the scale is known but the symbol is not", () => { + test("names the tracked symbol alongside the address (issue #323)", () => { + // The tracked entry supplies both halves now: the scale, and the + // symbol the output line is named by. Before #323 the symbol was read + // from the bundled list alone, so this line fell back to the address. const sources = { trackedTokens: [ { address: NOVEL_OUT, symbol: "NOVEL", decimals: 6 }, ], }; - expect(detail(data(), "Token Out", sources).value).toBe(NOVEL_OUT); + expect(detail(data(), "Token Out", sources).value).toBe( + "NOVEL (" + NOVEL_OUT + ")", + ); expect(detail(data(), "Min. received", sources).value).toBe( - "1000.0000", + "1000.0000 NOVEL", ); }); }); diff --git a/tests/uniswapUnknownDecimals.test.js b/tests/uniswapUnknownDecimals.test.js index a5be93c..c8ecc57 100644 --- a/tests/uniswapUnknownDecimals.test.js +++ b/tests/uniswapUnknownDecimals.test.js @@ -98,12 +98,13 @@ describe("a swap of a token outside the bundled list", () => { state.trackedTokens = [ { address: NOVEL, symbol: "NOVEL", decimals: 6 }, ]; - expect(swapDetail(data(), "Amount").value).toBe("1000.0000"); + // The tracked entry names the token as well as scaling it (issue #323). + expect(swapDetail(data(), "Amount").value).toBe("1000.0000 NOVEL"); }); test("shows the true quantity from the explorer's decimals", () => { state.wallets = walletsHolding(NOVEL, "6"); - expect(swapDetail(data(), "Amount").value).toBe("1000.0000"); + expect(swapDetail(data(), "Amount").value).toBe("1000.0000 NOVEL"); }); test("refuses to format when nothing knows the scale", () => { @@ -140,7 +141,7 @@ describe("the Min. received line takes the same rule", () => { { address: NOVEL_OUT, symbol: "NOVEL", decimals: 6 }, ]; const data = swapData(WETH, HALF_WETH, NOVEL_OUT, THOUSAND_AT_SIX); - expect(swapDetail(data, "Min. received").value).toBe("1000.0000"); + expect(swapDetail(data, "Min. received").value).toBe("1000.0000 NOVEL"); }); });