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(