From ca9cd438d977f84f12f23fbb6524b0fb030cd4a2 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 20 Oct 2026 04:16:04 +0200 Subject: [PATCH] fix: transaction screens name a token from the tokens of the transaction's network (closes #629) decodeCalldata() looked a token's symbol and decimals up in the active network's tracked tokens and balances, whatever chain id it was given, so TransactionDetail and TxApproval showed a token tracked on another network by address only, with its amount in base units. The lookup now uses what the wallet knows of the tokens on the transaction's network: the active network's tracked tokens and balances, or for another network the tokens tracked on it and no balances. TxApproval's token labels, the symbol it hands the wait, success and error screens, and SuccessTx's decoded token label use the same lookup. Model: opus-5-5 --- README.md | 33 ++--- src/popup/views/approval.js | 26 ++-- src/popup/views/helpers.js | 38 +++-- src/popup/views/txStatus.js | 2 +- tests/approvalTokenNetwork.test.js | 220 +++++++++++++++++++++++++++++ tests/tokenTransferDetail.test.js | 48 ++++++- tests/txStatus.test.js | 27 ++++ 7 files changed, 351 insertions(+), 43 deletions(-) create mode 100644 tests/approvalTokenNetwork.test.js diff --git a/README.md b/README.md index 31716d2..418bba9 100644 --- a/README.md +++ b/README.md @@ -1116,22 +1116,23 @@ with the same floor (`.5` reads `.5000`). **Specific Exception — unknown token scale:** Calldata carries base units and no scale, so every amount on the dApp approval screen needs the token's `decimals`. -It is resolved from the bundled token list, then from the tokens the user -tracks, then from what the block explorer reported for the contract -(`resolveTokenDecimals()` in `src/shared/approvalAmount.js`). Where none of them -answers, the amount is not formatted: the line reads -`5000000000 base units (decimals unknown)`. A guessed scale is not an -approximation but a different number — 1,000 units of a 6-decimal token -formatted at 18 decimals reads `0.000000001` — on the screen whose only job is -to state what is being authorized. Both amount paths of that screen take this -rule: the ERC-20 `transfer`/`approve` line (`src/popup/views/approval.js`) and -the swap's `Amount` and `Min. received` lines (`src/shared/uniswap.js`). The -token permission warning on the signature screen takes the same rule for its -amounts. An unbounded allowance or permit needs no scale to describe and is -still shown as `Unlimited`. A source's answer counts only if it is a whole -number from 0 to 80: `decimals()` returns a `uint8`, but `formatUnits()` cannot -format more than 80 decimal places, so a token that reports 81 to 255 is shown -as one whose scale nothing knows. +It is resolved from the bundled token list, then from the tokens the user tracks +on the network the transaction is for, then, when that is the active network, +from what the block explorer reported for the contract in the wallet's balances, +which are the active network's only (`resolveTokenDecimals()` in +`src/shared/approvalAmount.js`). Where none of them answers, the amount is not +formatted: the line reads `5000000000 base units (decimals unknown)`. A guessed +scale is not an approximation but a different number — 1,000 units of a +6-decimal token formatted at 18 decimals reads `0.000000001` — on the screen +whose only job is to state what is being authorized. Both amount paths of that +screen take this rule: the ERC-20 `transfer`/`approve` line +(`src/popup/views/approval.js`) and the swap's `Amount` and `Min. received` +lines (`src/shared/uniswap.js`). The token permission warning on the signature +screen takes the same rule for its amounts. An unbounded allowance or permit +needs no scale to describe and is still shown as `Unlimited`. A source's answer +counts only if it is a whole number from 0 to 80: `decimals()` returns a +`uint8`, but `formatUnits()` cannot format more than 80 decimal places, so a +token that reports 81 to 255 is shown as one whose scale nothing knows. 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 diff --git a/src/popup/views/approval.js b/src/popup/views/approval.js index 8f26992..6911927 100644 --- a/src/popup/views/approval.js +++ b/src/popup/views/approval.js @@ -11,6 +11,7 @@ const { onViewLeave, formatFee, tokenLabel, + tokenSourcesByChainId, } = require("./helpers"); const { state, saveState, currentNetwork } = require("../../shared/state"); const { @@ -85,13 +86,12 @@ function tokenAmountText(rawAmount, decimals, symbol) { function decodeCalldata(data, toAddress, chainId, fromAddress) { if (!data || data === "0x" || data.length < 10) return null; - // Where a token's scale is looked for, for every decoder below: the ERC-20 - // amount line and the swap's Amount and Min. received lines resolve it the - // same way, and refuse to format the same way when it is nowhere. - const decimalsSources = { - trackedTokens: state.trackedTokens, - wallets: state.wallets, - }; + // Where a token's name and scale are looked for, for every decoder below: + // what the wallet knows of the tokens on the network with `chainId`, which + // need not be the active one. The ERC-20 amount line and the swap's Amount + // and Min. received lines resolve the scale the same way, and refuse to + // format the same way when it is nowhere. + const decimalsSources = tokenSourcesByChainId(chainId); // Try ERC-20 (approve / transfer) try { @@ -261,10 +261,7 @@ function showTxApproval(details) { const toAddr = approvedTx.to; const ethValue = formatEther(approvedTx.value || "0"); - const sources = { - trackedTokens: state.trackedTokens, - wallets: state.wallets, - }; + const sources = tokenSourcesByChainId(approvedTx.chainId); // Build txInfo for status screens pendingTxDetails = { @@ -344,8 +341,9 @@ function showTxApproval(details) { $("approve-tx-origin").textContent = details.origin; $("approve-tx-from").innerHTML = approvalAddressHtml(details.approvedFrom); - // Show token symbol next to contract address if known - const symbol = toAddr ? tokenLabel(toAddr) : null; + // Show token symbol next to contract address if known on the network the + // transaction is for + const symbol = toAddr ? tokenLabel(toAddr, approvedTx.chainId) : null; if (toAddr) { let toHtml = ""; if (symbol) { @@ -388,7 +386,7 @@ function showTxApproval(details) { detailsHtml += `
${escapeHtml(d.label)}
`; if (d.address) { if (d.isToken) { - detailsHtml += `
${escapeHtml(tokenLabel(d.address) || "Unknown token")}
`; + detailsHtml += `
${escapeHtml(tokenLabel(d.address, approvedTx.chainId) || "Unknown token")}
`; } detailsHtml += approvalAddressHtml(d.address); } else { diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index b14e461..cd9fa8d 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -1,7 +1,7 @@ // Helpers the views in this directory share: showing a view and going back, // error and flash lines and the warning banners, formatting of amounts, fees, -// balances, addresses and dates, the RPC and Blockscout URLs and node of the -// network a chain id names, explorer links, and click-to-copy. +// balances, addresses and dates, the RPC and Blockscout URLs, node and tokens +// of the network a chain id names, explorer links, and click-to-copy. // // Escaping rule for every view in this directory, since they all build // markup by concatenation: any VALUE interpolated into an innerHTML string @@ -300,15 +300,30 @@ function nativeCurrency() { return currentNetwork().nativeCurrency; } -// 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) { - return resolveTokenSymbol(address, { - trackedTokens: state.trackedTokens, - wallets: state.wallets, - }); +// What the wallet knows of the tokens on the network with this chain id, or +// on the active network when no chain id is given, as the `sources` that +// resolveTokenSymbol() and resolveTokenDecimals() take. A contract address +// names a token on one network only, and every token balance held is the +// active network's, so another network gets the tokens tracked on it +// (state.networkTrackedTokens, see applyChainSwitchFields()) and no balances. +function tokenSourcesByChainId(chainId) { + const net = chainId ? networkByChainId(chainId) : currentNetwork(); + if (net && net.id === state.networkId) { + return { trackedTokens: state.trackedTokens, wallets: state.wallets }; + } + return { + trackedTokens: (net && state.networkTrackedTokens[net.id]) || [], + wallets: [], + }; +} + +// The symbol shown for a token line on the network with `chainId`, or on the +// active network when none is given: from the bundled list, the tokens the +// user tracks, and the explorer's report, the sources 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, chainId) { + return resolveTokenSymbol(address, tokenSourcesByChainId(chainId)); } // A network fee in wei as the confirmation and approval screens both show it: @@ -775,6 +790,7 @@ module.exports = { keyMaterialName, unknownableAmount, nativeCurrency, + tokenSourcesByChainId, tokenLabel, formatFee, addressDotHtml, diff --git a/src/popup/views/txStatus.js b/src/popup/views/txStatus.js index f6fcfb3..e55913f 100644 --- a/src/popup/views/txStatus.js +++ b/src/popup/views/txStatus.js @@ -280,7 +280,7 @@ function decodedDetailsHtml(decoded, chainId) { html += `
${escapeHtml(d.label)}
`; if (d.address) { if (d.isToken) { - const sym = tokenLabel(d.address) || "Unknown token"; + const sym = tokenLabel(d.address, chainId) || "Unknown token"; html += `
${escapeHtml(sym)}
`; html += toAddressHtml(d.address, chainId); } else { diff --git a/tests/approvalTokenNetwork.test.js b/tests/approvalTokenNetwork.test.js new file mode 100644 index 0000000..20ed1dc --- /dev/null +++ b/tests/approvalTokenNetwork.test.js @@ -0,0 +1,220 @@ +// The approval screen for a site's Sepolia token transfer while Ethereum +// Mainnet is active (https://git.eeqj.de/sneak/AutistMask/issues/629). +// +// The screen named the token, and scaled its amount, from the active network's +// tracked tokens and balances, so a token the wallet tracks on Sepolia read as +// an unknown token with its amount in base units, and the screens after it +// labelled the amount `?`. It now uses what the wallet knows of the tokens on +// the network the transaction is for. +// +// Driven through the real approval screen, password and Confirm included, +// against a minimal DOM stub in the shape tests/swapStatusAmount.test.js uses. +// The vault is mocked so that no password has to be hashed. + +jest.mock("../src/shared/vault", () => ({ + decryptWithPassword: jest.fn(), +})); + +globalThis.chrome = { + storage: { local: { get: async () => ({}), set: async () => {} } }, +}; + +const { Interface } = require("ethers"); +const { state } = require("../src/shared/state"); +const { NETWORKS } = require("../src/shared/networks"); +const { decryptWithPassword } = require("../src/shared/vault"); +const approval = require("../src/popup/views/approval"); +const txStatus = require("../src/popup/views/txStatus"); + +// A well-known test phrase, and its first address. +const PHRASE = "test test test test test test test test test test test junk"; +const FROM = "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266"; +const RECIPIENT = "0x711e691962E5c054b78071C789A0C8CE657377aD"; +// A token the bundled token list does not name. +const TOKEN = "0x74de5d4FCbf63E00296fd95d33236B9794016631"; + +// FROM sends 1234.5 of the 6-decimal token to RECIPIENT. +const TRANSFER_DATA = new Interface([ + "function transfer(address to, uint256 amount)", +]).encodeFunctionData("transfer", [RECIPIENT, 1234500000n]); + +function makeElement(id) { + const classes = new Set(); + const el = { + id, + textContent: "", + value: "", + innerHTML: "", + disabled: false, + style: {}, + dataset: {}, + listeners: {}, + classList: { + add: (...names) => names.forEach((n) => classes.add(n)), + remove: (...names) => names.forEach((n) => classes.delete(n)), + contains: (n) => classes.has(n), + toggle: (n, force) => { + const on = force === undefined ? !classes.has(n) : force; + if (on) classes.add(n); + else classes.delete(n); + return on; + }, + }, + addEventListener: (name, fn) => { + el.listeners[name] = el.listeners[name] || []; + el.listeners[name].push(fn); + }, + querySelectorAll: () => [], + appendChild: () => {}, + }; + // Views reach for .parentElement to hide whole sections. + Object.defineProperty(el, "parentElement", { + get: () => node(id + "-parent"), + }); + return el; +} + +function makeDocument() { + const els = new Map(); + return { + getElementById(id) { + // The debug banner is created on demand by helpers.js; absent + // is the state a non-debug, non-testnet popup is in. + if (id === "debug-banner") return null; + if (!els.has(id)) els.set(id, makeElement(id)); + return els.get(id); + }, + createElement: () => makeElement("created"), + body: { prepend: () => {} }, + }; +} + +function node(id) { + return globalThis.document.getElementById(id); +} + +function click(id) { + return Promise.all((node(id).listeners.click || []).map((fn) => fn())); +} + +// One line of the decoded details: its label and what is drawn under it. +function decodedLine(label) { + return node("approve-tx-decoded-details") + .innerHTML.split('
') + .find((line) => + line.startsWith(`
${label}
`), + ); +} + +// Open the approval screen, with Ethereum Mainnet active and FROM holding +// `tokenBalances` there, for the site's Sepolia transaction sending +// TRANSFER_DATA to the token. Confirm answers the signed transaction with a +// failure, which leads to the error screen. +async function showSepoliaTransfer(tokenBalances = []) { + globalThis.document = makeDocument(); + globalThis.window = { location: { search: "" }, close: () => {} }; + globalThis.chrome.runtime = { + connect: () => ({ postMessage: () => {} }), + sendMessage: (msg, reply) => { + if (!reply) return; + if (msg.type !== "AUTISTMASK_GET_APPROVAL") { + return reply({ + error: "The transaction could not be sent.", + retryable: false, + }); + } + reply({ + type: "tx", + origin: "https://dapp.example", + isPhishingDomain: false, + approvedFrom: FROM, + approvedTx: { + type: 2, + from: FROM, + chainId: NETWORKS.sepolia.chainId, + nonce: "0x7", + gasLimit: "0x30d40", + maxPriorityFeePerGas: "0x3b9aca00", + maxFeePerGas: "0x77359400", + to: TOKEN, + value: "0x0", + data: TRANSFER_DATA, + accessList: [], + }, + }); + }, + }; + state.networkId = "mainnet"; + state.activeAddress = FROM; + state.wallets = [ + { + type: "hd", + name: "Wallet 1", + xpub: "xpub-wallet-1", + encryptedSecret: "encrypted-secret-1", + nextIndex: 1, + addresses: [ + { address: FROM, index: 0, balance: "0", tokenBalances }, + ], + }, + ]; + state.currentView = null; + txStatus.init({ doRefreshAndRender: () => {} }); + approval.init({}); + await approval.show(1); +} + +async function confirm() { + decryptWithPassword.mockResolvedValue(PHRASE); + node("approve-tx-password").value = "any password"; + await click("btn-approve-tx"); +} + +beforeEach(() => { + state.trackedTokens = []; + state.networkTrackedTokens = {}; +}); + +describe("a token tracked on Sepolia only", () => { + beforeEach(() => { + state.networkTrackedTokens = { + sepolia: [{ address: TOKEN, symbol: "TDOL", decimals: 6 }], + }; + }); + + test("is named on the Contract line and the decoded Token line, with its amount at its scale", async () => { + await showSepoliaTransfer(); + expect(node("approve-tx-to").innerHTML).toContain( + '
TDOL
', + ); + expect(decodedLine("Token")).toContain( + '
TDOL
', + ); + expect(decodedLine("Amount")).toContain( + '
1234.5000 TDOL
', + ); + }); + + test("labels the amount on the error screen that follows", async () => { + await showSepoliaTransfer(); + await confirm(); + expect(state.currentView).toBe("error-tx"); + expect(node("error-tx-summary").textContent).toBe("1234.5000 TDOL"); + }); +}); + +// The same contract address on mainnet is another token, so what the wallet +// tracks and holds there names nothing on Sepolia. +test("a token tracked and held on mainnet only is not named, and its amount is in base units", async () => { + state.trackedTokens = [{ address: TOKEN, symbol: "MDOL", decimals: 18 }]; + await showSepoliaTransfer([ + { address: TOKEN, symbol: "MDOL", decimals: 18, balance: "1.0" }, + ]); + expect(node("approve-tx-to").innerHTML).not.toContain("MDOL"); + expect(decodedLine("Token")).toContain( + '
Unknown token
', + ); + expect(decodedLine("Amount")).toContain( + '
1234500000 base units (decimals unknown)
', + ); +}); diff --git a/tests/tokenTransferDetail.test.js b/tests/tokenTransferDetail.test.js index e7b3b54..9ac9d9b 100644 --- a/tests/tokenTransferDetail.test.js +++ b/tests/tokenTransferDetail.test.js @@ -12,7 +12,9 @@ // whichever network is active // (https://git.eeqj.de/sneak/AutistMask/issues/584), and the transaction is // read from that network's Blockscout, the one Settings has for it -// (https://git.eeqj.de/sneak/AutistMask/issues/623). +// (https://git.eeqj.de/sneak/AutistMask/issues/623). Its decoded details name +// the token, and scale its amount, from what the wallet knows of that +// network's tokens (https://git.eeqj.de/sneak/AutistMask/issues/629). // // The row is the one the real history parser makes from the explorer's answer, // opened through transactionDetail.show(), which is what a click on the row @@ -165,6 +167,7 @@ beforeEach(() => { }, ]; state.trackedTokens = [{ address: TOKEN, symbol: "TDOL", decimals: 6 }]; + state.networkTrackedTokens = {}; state.viewData = {}; state.viewStack = []; state.currentView = null; @@ -245,3 +248,46 @@ test("with mainnet active and no Blockscout remembered for Sepolia, a Sepolia tr NETWORKS.sepolia.defaultBlockscoutUrl + "/transactions/" + TX_HASH, ]); }); + +// Open the Sepolia transfer's detail screen with mainnet active, and wait for +// its decoded details. +async function showSepoliaTransferWithMainnetActive() { + state.networkId = "mainnet"; + state.blockscoutUrl = OTHER_EXPLORER; + state.networkEndpoints = { sepolia: { blockscoutUrl: EXPLORER } }; + const [row] = await fetchRecentTransactions( + HOLDER, + EXPLORER, + NETWORKS.sepolia.chainId, + ); + transactionDetail.show(row); + for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0)); +} + +test("with mainnet active, a Sepolia token transfer's decoded details name the token tracked on Sepolia and its amount at its scale", async () => { + state.trackedTokens = []; + state.networkTrackedTokens = { + sepolia: [{ address: TOKEN, symbol: "TDOL", decimals: 6 }], + }; + await showSepoliaTransferWithMainnetActive(); + + expect(decodedLine("Token")).toContain('
TDOL
'); + expect(decodedLine("Amount")).toContain( + '
1234.5000 TDOL
', + ); +}); + +// The same contract address on mainnet is another token, so what the wallet +// tracks and holds there names nothing on Sepolia. +test("with mainnet active, a Sepolia token transfer's decoded details take nothing from mainnet's tracked tokens or balances", async () => { + state.trackedTokens = [{ address: TOKEN, symbol: "MDOL", decimals: 18 }]; + state.wallets[0].addresses[0].tokenBalances = [ + { address: TOKEN, symbol: "MDOL", decimals: 18, balance: "1.0" }, + ]; + await showSepoliaTransferWithMainnetActive(); + + expect(decodedLine("Token")).not.toContain("MDOL"); + expect(decodedLine("Amount")).toContain( + '
1234500000 base units (decimals unknown)
', + ); +}); diff --git a/tests/txStatus.test.js b/tests/txStatus.test.js index a78e9b6..f2ce665 100644 --- a/tests/txStatus.test.js +++ b/tests/txStatus.test.js @@ -150,6 +150,8 @@ beforeEach(() => { state.rpcUrl = NETWORKS.mainnet.defaultRpcUrl; state.networkEndpoints = {}; state.wallets = []; + state.trackedTokens = []; + state.networkTrackedTokens = {}; state.viewData = {}; state.viewStack = []; state.currentView = null; @@ -627,6 +629,31 @@ describe("SuccessTx and ErrorTx for a Sepolia transaction with mainnet active", expectSepoliaLinks(); }); + // https://git.eeqj.de/sneak/AutistMask/issues/629 + test("SuccessTx's decoded action names the token from the tokens tracked on Sepolia", async () => { + function expectTokenNamed() { + expect(getElement("success-tx-decoded").innerHTML).toContain( + '
TDOL
', + ); + } + state.networkTrackedTokens = { + sepolia: [{ address: TOKEN, symbol: "TDOL", decimals: 6 }], + }; + const decoded = { + name: "Transfer", + details: [{ label: "Token", address: TOKEN, isToken: true }], + }; + mockReceiptLookup.mockResolvedValue({ blockNumber: 21000000 }); + txStatus.showWait({ ...sepoliaTx, decoded }, TX_HASH); + await jest.advanceTimersByTimeAsync(10000); + expect(state.currentView).toBe("success-tx"); + expectTokenNamed(); + + elements.clear(); + txStatus.renderSuccess(); + expectTokenNamed(); + }); + test("the recipient and hash on ErrorTx link to the Sepolia explorer", () => { function expectSepoliaLinks() { expect(getElement("error-tx-to").innerHTML).toContain(