diff --git a/README.md b/README.md index a393165..ea4f247 100644 --- a/README.md +++ b/README.md @@ -17,11 +17,11 @@ don't implement any crypto, and don't send user-specific data anywhere but a (user-configurable) Ethereum RPC endpoint (which defaults to a public node). The extension contacts three user-configurable services: the configured RPC node for blockchain interactions, a public CoinDesk API (no API key) for realtime price -information, and a Blockscout block-explorer API for transaction history and -token balances. It also performs best-effort Etherscan address label lookups -during transaction confirmation. A community-maintained phishing domain -blocklist is built into the extension at build time and checked locally; nothing -is fetched for it at runtime. +information, and a Blockscout block-explorer API for transaction history and the +balances of tokens the user has not tracked. It also performs best-effort +Etherscan address label lookups during transaction confirmation. A +community-maintained phishing domain blocklist is built into the extension at +build time and checked locally; nothing is fetched for it at runtime. In the extension is a hardcoded list of the top ERC20 contract addresses. You can add any ERC20 contract by contract address if you wish, but the hardcoded @@ -845,7 +845,7 @@ src/ views/ — one JS module per screen (home, send, approval, etc.) shared/ — modules used by both popup and background alarms.js — recurring background jobs (extension alarms API) - balances.js — ETH + ERC-20 balance fetching via RPC + Blockscout + balances.js — ETH and tracked token balances via RPC, others via Blockscout constants.js — chain IDs, default RPC endpoint, ERC-20 ABI ens.js — ENS forward/reverse resolution (popup only) holders.js — holder-count parsing and the low-holder rule @@ -1155,13 +1155,14 @@ printing `0.0000` for money that is really there. `0` is a real scale and is never treated as absent. `tokenBalances[].decimals` is therefore the explorer's own answer and nothing -else, which is not the same question as the scale a screen should render at. -Anything that needs the second one calls `resolveTokenDecimals()` — the balance -list, the approval and swap lines, and the Send screen, which carries the -resolved scale onto the pending transaction for `transferAmount.js` to encode -and compare against. Reading the stored field directly instead answers `null` -for a bundled or tracked token the explorer merely omitted, which is not a -refusal the wallet has any reason to make. +else, and `null` in the entry of a tracked token, whose balance is read through +the RPC endpoint and not asked of the explorer. It is not the same question as +the scale a screen should render at. Anything that needs the second one calls +`resolveTokenDecimals()` — the balance list, the approval and swap lines, and +the Send screen, which carries the resolved scale onto the pending transaction +for `transferAmount.js` to encode and compare against. Reading the stored field +directly instead answers `null` for a bundled or tracked token the explorer +merely omitted, which is not a refusal the wallet has any reason to make. The Send screen also consults every address's explorer reports, so a contract two addresses report different `decimals` for has no scale there, and the stored @@ -1379,25 +1380,31 @@ The core hierarchy is **Wallets → Addresses**: - The user can have multiple wallets, each with multiple addresses (HD) or a single address (key). -Which tokens an address shows is decided by `fetchTokenBalances()` in -`src/shared/balances.js`, from the Blockscout `token-balances` response, so -tokens do appear without the user adding them. An ERC-20 is shown when its -balance is nonzero and it is in the bundled known-token list, is tracked by the -user, or has 1,000 or more holders; a token claiming a symbol from the bundled -list from any other contract address is always dropped, and so is any token -claiming a symbol that belongs to the native asset and therefore has no -legitimate contract at all (`"ETH"`, and every network's `nativeCurrency`, such -as `"SepoliaETH"`, on every network). That filter is unconditional — the "Hide -tokens with fewer than 1,000 holders" setting governs the transaction history -and the send-screen token selector, not this list. A token's holder count is -unknown when the explorer reports none, or reports anything other than a whole -number written in digits alone, such as `1,000` or `1e3` (`parseHoldersCount()` -in `src/shared/holders.js`). This list does not take an unknown count as 1,000 -or more, so such a token is shown only when it is on the bundled list or -tracked. `fetchTokenBalances()` stores every nonzero holding of a token it -admits, however small, but a holding below 0.000001 is left out of the balance -lists, the send-screen token selector, the address total and the remove-address -warning (`isBelowOneMillionth()` in `src/shared/amountDisplay.js`). The Send and +Which tokens an address shows is decided on every refresh by `refreshBalances()` +in `src/shared/balances.js`. The balance of each token tracked on the active +network is read with `balanceOf` through the RPC endpoint, for every address; a +read that fails leaves that token's previous figure, as a failed ETH balance +does. Every other token comes from the Blockscout `token-balances` response +(`fetchTokenBalances()`), so tokens do appear without the user adding them, and +they keep their previous figures while Blockscout fails. A tracked token is +shown when its balance is nonzero, and any other ERC-20 when its balance is +nonzero and it is in the bundled known-token list or has 1,000 or more holders. +A token claiming a symbol from the bundled list from any other contract address +is always dropped, tracked or not, and so is any token claiming a symbol that +belongs to the native asset and therefore has no legitimate contract at all +(`"ETH"`, and every network's `nativeCurrency`, such as `"SepoliaETH"`, on every +network). That filter is unconditional — the "Hide tokens with fewer than 1,000 +holders" setting governs the transaction history and the send-screen token +selector, not this list. A token's holder count is unknown when the explorer +reports none, or reports anything other than a whole number written in digits +alone, such as `1,000` or `1e3` (`parseHoldersCount()` in +`src/shared/holders.js`); a tracked token's entry has none, because the explorer +is not asked about it. This list does not take an unknown count as 1,000 or +more, so such a token is shown only when it is on the bundled list or tracked. +Every nonzero holding of a token shown is stored exactly, however small, but a +holding below 0.000001 is left out of the balance lists, the send-screen token +selector, the address total and the remove-address warning +(`isBelowOneMillionth()` in `src/shared/amountDisplay.js`). The Send and confirmation screens show it when its token is the one being sent. Tracked tokens with a zero balance are listed as well while "Show tracked tokens with zero balance" is on. @@ -1781,7 +1788,8 @@ view would leave a wallet one click from deletion. etherscan link) plus name, symbol, decimals, holder count and project website where known. The "Holders:" row is left out, not shown as 0, when the token's balance-list entry has no holder count: the explorer did not - report a readable one, or the token is not in the balance list + report a readable one, the token is tracked, or the token is not in the + balance list - Token-filtered transaction list (only this token's transfers) - **Transitions**: - "Send" → **Send** (token locked: the dropdown is replaced by a static @@ -2662,7 +2670,8 @@ AutistMask is not a fully self-contained offline tool. It necessarily communicates with three external services to function as a wallet: - **Ethereum JSON-RPC endpoint**: The extension needs an Ethereum node to query - balances (`eth_getBalance`), read ERC-20 token contracts (`eth_call`), + balances (`eth_getBalance`), read ERC-20 token contracts (`eth_call`), among + them the balance of every tracked token at every address (`balanceOf`), estimate gas (`eth_estimateGas`), fetch nonces (`eth_getTransactionCount`), broadcast transactions (`eth_sendRawTransaction`), and check transaction receipts. The default endpoint is a public RPC (configurable by the user to @@ -2680,9 +2689,10 @@ communicates with three external services to function as a wallet: addresses or user-specific data. - **Blockscout block-explorer API**: Used to fetch transaction history (normal - transactions and ERC-20 token transfers), ERC-20 token balances, and token - holder counts (for spam filtering). The default endpoint is - `https://eth.blockscout.com/api/v2` (configurable by the user in Settings). + transactions and ERC-20 token transfers), the balances of ERC-20 tokens the + user has not tracked, and token holder counts (for spam filtering). The + default endpoint is `https://eth.blockscout.com/api/v2` (configurable by the + user in Settings). - **Data sent**: Ethereum addresses. Blockscout receives the user's addresses to query their transaction history and token balances. No private keys, passwords, or signing operations are sent. @@ -2989,8 +2999,10 @@ indexes it as a real token transfer. excluded from the token selector on the send screen. This prevents users from accidentally interacting with a spoofed token that appeared in their balance via a fake Transfer event. A token whose holder count is unknown is kept in - the selector. The selector offers only tokens in the balance list, so such a - token is one on the bundled list or one the user tracks. + the selector, and so is every token the user tracks, whose entry carries no + holder count (see Data Model). The selector offers only tokens in the balance + list, so a token with an unknown count is one on the bundled list or one the + user tracks. - **Dust transaction filtering**: A second wave of the same attack used real native ETH transfers instead of fake tokens. Transaction diff --git a/src/shared/balances.js b/src/shared/balances.js index 7c16604..fcba792 100644 --- a/src/shared/balances.js +++ b/src/shared/balances.js @@ -1,5 +1,6 @@ -// Balance fetching: ETH balances via RPC, ERC-20 token balances via -// Blockscout, ENS reverse lookup via RPC. +// Balance fetching: ETH balances and the balances of the tokens the user +// tracks via RPC, the other ERC-20 token balances via Blockscout, ENS reverse +// lookup via RPC. const { JsonRpcProvider, @@ -81,7 +82,9 @@ function rawUnits(value) { // Fetch token balances for a single address from Blockscout. // 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. +// explicitly tracked by the user, or have >= 1000 holders. refreshBalances() +// keeps only the rows for tokens the user has not tracked: a tracked token's +// balance is read through the RPC endpoint (fetchTrackedTokenBalances()). // // `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 @@ -205,8 +208,77 @@ async function fetchTokenBalances( } } -// Fetch ETH balances, ENS names, and ERC-20 token balances for all addresses. +// The balance at `address` of each token the user tracks, read with balanceOf +// through the RPC endpoint. Returns rows shaped as fetchTokenBalances() returns +// them, for the tokens the address holds. +// +// A token whose balanceOf fails keeps its row from `previous`, the address's +// rows before this refresh, as a failed ETH balance keeps its figure, and the +// failure is logged. A balance of zero gets no row, as a zero Blockscout +// reports gets none: the balance lists take a tracked token's zero from the +// tracked list. A token whose symbol the spoof rule refuses gets no row either, +// as in fetchTokenBalances(). +// +// The balance is stated at the scale the screens read this token at +// (resolveTokenDecimals()): the bundled list's, then the one the tracked token +// records. `decimals` and `holders` hold what Blockscout reported for a token; +// Blockscout is not asked about a tracked token, so both are null. +// // `signal` is as for fetchTokenBalances(). +async function fetchTrackedTokenBalances( + provider, + address, + trackedTokens, + previous, + signal, +) { + const rows = await Promise.all( + trackedTokens.map(async (token) => { + if (isSpoofedSymbol(token.symbol, token.address)) return null; + const lower = token.address.toLowerCase(); + let raw; + try { + const contract = new Contract( + token.address, + ERC20_ABI, + provider, + ); + raw = await contract.balanceOf(address); + } catch (e) { + if (!signal?.aborted) { + log.errorf( + "token balance failed", + address, + token.address, + e.shortMessage || e.message, + ); + } + return ( + previous.find( + (row) => String(row.address).toLowerCase() === lower, + ) || null + ); + } + if (raw === 0n) return null; + const scale = resolveTokenDecimals(lower, { trackedTokens }); + return { + address: token.address, + name: token.name || "", + symbol: token.symbol || "???", + decimals: null, + // null when nothing knows the scale: a holding whose amount + // cannot be stated, never a zero. + balance: scale === null ? null : formatTokenBalance(raw, scale), + holders: null, + }; + }), + ); + return rows.filter((row) => row !== null); +} + +// Fetch ETH balances, ENS names, and ERC-20 token balances for all addresses. +// `trackedTokens` are the tokens tracked on the active network. `signal` is as +// for fetchTokenBalances(). async function refreshBalances( wallets, rpcUrl, @@ -217,6 +289,8 @@ async function refreshBalances( ) { log.debugf("refreshBalances start, rpc:", urlOrigin(rpcUrl)); const provider = getProvider(rpcUrl, networkId); + const tracked = trackedTokens || []; + const trackedSet = new Set(tracked.map((t) => t.address.toLowerCase())); const updates = []; for (const wallet of wallets) { @@ -266,23 +340,38 @@ async function refreshBalances( }), ); - // ERC-20 token balances via Blockscout + // ERC-20 token balances: the tracked tokens' through the RPC + // endpoint, and from Blockscout only the tokens the user has not + // tracked. When Blockscout fails, the untracked rows from before + // this refresh stay. + const previous = addr.tokenBalances || []; updates.push( - fetchTokenBalances( - addr.address, - blockscoutUrl, - trackedTokens, - signal, - ).then((balances) => { - if (balances !== null) { - addr.tokenBalances = balances; - log.debugf( - "Token balances", - addr.address, - balances.length, - "tokens", - ); - } + Promise.all([ + fetchTrackedTokenBalances( + provider, + addr.address, + tracked, + previous, + signal, + ), + fetchTokenBalances( + addr.address, + blockscoutUrl, + tracked, + signal, + ), + ]).then(([trackedRows, explorerRows]) => { + const untrackedRows = (explorerRows ?? previous).filter( + (row) => + !trackedSet.has(String(row.address).toLowerCase()), + ); + addr.tokenBalances = trackedRows.concat(untrackedRows); + log.debugf( + "Token balances", + addr.address, + addr.tokenBalances.length, + "tokens", + ); }), ); } diff --git a/tests/balanceRefreshCancelled.test.js b/tests/balanceRefreshCancelled.test.js index 5852484..9686fa8 100644 --- a/tests/balanceRefreshCancelled.test.js +++ b/tests/balanceRefreshCancelled.test.js @@ -18,6 +18,11 @@ const { const RPC_URL = "https://rpc.example.invalid"; const EXPLORER_URL = "https://explorer.example.invalid/api/v2"; const ADDRESS = "0x1111111111111111111111111111111111111111"; +const TRACKED = { + address: "0x7777777777777777777777777777777777777777", + symbol: "TRK", + decimals: 6, +}; const realFetch = globalThis.fetch; let logged; @@ -30,9 +35,9 @@ beforeEach(() => { const failedToFetch = async () => { throw new TypeError("Failed to fetch"); }; - // The RPC calls (ETH balance, ENS name, and the scan's balance and - // transaction count) and the explorer request (token balances) all fail - // the same way. + // The RPC calls (ETH balance, ENS name, the tracked token's balance, and + // the scan's balance and transaction count) and the explorer request (the + // other token balances) all fail the same way. FetchRequest.registerGetUrl(failedToFetch); globalThis.fetch = jest.fn(failedToFetch); }); @@ -49,7 +54,7 @@ function refresh(signal) { wallets, RPC_URL, EXPLORER_URL, - [], + [TRACKED], "mainnet", signal, ); @@ -60,6 +65,7 @@ test("a failure while the popup is open is reported", async () => { for (const label of [ "ETH balance failed", "ENS reverse failed", + "token balance failed", "fetchTokenBalances failed: Failed to fetch", ]) { expect(logged.some((line) => line.includes(label))).toBe(true); diff --git a/tests/e2e/run.js b/tests/e2e/run.js index 2d95c8f..651bea5 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -3480,6 +3480,24 @@ function hostileSymbolState(page, tokenAddress) { } test("a token whose symbol() returns markup renders as text (#307)", async (env) => { + // The stub token is no longer tracked from here on, so the explorer is + // what lists it, as it lists a token airdropped to the victim. A tracked + // token is listed from the RPC endpoint under the symbol it was added + // with, which the hostile symbol below never reaches + // (https://git.eeqj.de/sneak/AutistMask/issues/669). + await env.page.click("#btn-settings"); + await visible(env.page, "#view-settings"); + await env.page.click("#settings-tracked-tokens .btn-remove-token"); + // The list is drawn again once the removal is saved. + await env.page.waitForFunction( + () => + document + .getElementById("settings-tracked-tokens") + .innerText.trim() === "None", + ); + await env.page.click("#btn-settings-back"); + await visible(env.page, "#view-address"); + env.routeOpts.ethBalanceWei = toHexWei(FUNDED_ETH_WEI); env.routeOpts.seedTokenBalance = true; env.routeOpts.tokenSymbolOverride = HOSTILE_SYMBOL; diff --git a/tests/trackedTokenBalances.test.js b/tests/trackedTokenBalances.test.js new file mode 100644 index 0000000..9bd9073 --- /dev/null +++ b/tests/trackedTokenBalances.test.js @@ -0,0 +1,236 @@ +// The balance of each token the user tracks is read with balanceOf through the +// RPC endpoint, for every address, and Blockscout's answer adds only the tokens +// the user has not tracked (https://git.eeqj.de/sneak/AutistMask/issues/669). +// +// The real refreshBalances() runs against a fake JSON-RPC node, answered +// through ethers' own request hook, and a fake Blockscout, answered through +// fetch(). + +const { AbiCoder, FetchRequest } = require("ethers"); +const { refreshBalances } = require("../src/shared/balances"); + +const RPC_URL = "https://rpc.example.invalid"; +const EXPLORER_URL = "https://explorer.example.invalid/api/v2"; +const FIRST = "0x1111111111111111111111111111111111111111"; +const SECOND = "0x2222222222222222222222222222222222222222"; + +// A token the user tracks, outside the bundled list, at 6 decimals. +const TOKEN = "0x7777777777777777777777777777777777777777"; +const TRACKED = { + address: TOKEN, + symbol: "TRK", + name: "Tracked Token", + decimals: 6, +}; + +// USDC, from the bundled list, which the user does not track. +const USDC = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48"; +// Neither on the bundled list nor tracked. +const OTHER = "0x8888888888888888888888888888888888888888"; + +const ZERO_WORD = "0x" + "0".repeat(64); +const BALANCE_OF = "0x70a08231"; + +// What balanceOf answers, by token contract and then by holder. A pair that is +// not here makes balanceOf fail. +let holdings; +// What Blockscout answers for every address; null makes the request fail. +let explorerRows; +let logged; + +// One JSON-RPC answer. A call that is not balanceOf is the ENS reverse lookup, +// answered with no resolver, so no name. +function reply(req) { + const envelope = { jsonrpc: "2.0", id: req.id }; + if (req.method === "eth_getBalance") { + return { ...envelope, result: "0x0" }; + } + if (req.method !== "eth_call") { + return { + ...envelope, + error: { code: -32601, message: "method not found" }, + }; + } + const { to, data } = req.params[0]; + if (!data.startsWith(BALANCE_OF)) return { ...envelope, result: ZERO_WORD }; + const holder = "0x" + data.slice(-40); + const amount = (holdings[to.toLowerCase()] || {})[holder]; + if (amount === undefined) { + return { + ...envelope, + error: { code: -32000, message: "fixture: balanceOf failed" }, + }; + } + return { + ...envelope, + result: AbiCoder.defaultAbiCoder().encode(["uint256"], [amount]), + }; +} + +const realFetch = globalThis.fetch; + +beforeEach(() => { + holdings = {}; + explorerRows = null; + logged = []; + jest.spyOn(console, "error").mockImplementation((...args) => { + logged.push(args.map(String).join(" ")); + }); + FetchRequest.registerGetUrl(async (req) => { + const body = JSON.parse(Buffer.from(req.body).toString("utf8")); + const answer = Array.isArray(body) ? body.map(reply) : reply(body); + return { + statusCode: 200, + statusMessage: "OK", + headers: { "content-type": "application/json" }, + body: Buffer.from(JSON.stringify(answer)), + }; + }); + globalThis.fetch = jest.fn(async () => { + if (explorerRows === null) throw new TypeError("Failed to fetch"); + return { ok: true, status: 200, json: async () => explorerRows }; + }); +}); + +afterEach(() => { + FetchRequest.registerGetUrl(FetchRequest.createGetUrlFunc()); + globalThis.fetch = realFetch; + jest.restoreAllMocks(); +}); + +// One Blockscout token-balances row. +function explorerRow(address, symbol, value, holders = "50000") { + return { + value: String(value), + token: { + type: "ERC-20", + address_hash: address, + symbol: symbol, + name: symbol, + decimals: "6", + holders_count: holders, + }, + }; +} + +// The row refreshBalances() writes for TOKEN from a balanceOf answer. +function trackedRow(balance) { + return { + address: TOKEN, + name: "Tracked Token", + symbol: "TRK", + decimals: null, + balance: balance, + holders: null, + }; +} + +async function refresh(addresses, trackedTokens) { + await refreshBalances( + [{ addresses }], + RPC_URL, + EXPLORER_URL, + trackedTokens, + "mainnet", + new AbortController().signal, + ); + return addresses; +} + +test("with Blockscout failing, a tracked token's balance is read through the RPC endpoint for every address", async () => { + holdings[TOKEN] = { [FIRST]: 1234500000n, [SECOND]: 1n }; + const [first, second] = await refresh( + [ + { address: FIRST, tokenBalances: [] }, + { address: SECOND, tokenBalances: [] }, + ], + [TRACKED], + ); + expect(first.tokenBalances).toEqual([trackedRow("1234.5")]); + // Exact, never cut: one base unit is not zero. + expect(second.tokenBalances).toEqual([trackedRow("0.000001")]); +}); + +test("with Blockscout failing, the untracked rows from before stay", async () => { + holdings[TOKEN] = { [FIRST]: 2000000n }; + const usdc = { + address: USDC, + name: "USD Coin", + symbol: "USDC", + decimals: 6, + balance: "5.0", + holders: 50000, + }; + const [first] = await refresh( + [{ address: FIRST, tokenBalances: [usdc] }], + [TRACKED], + ); + expect(first.tokenBalances).toEqual([trackedRow("2.0"), usdc]); +}); + +test("a tracked token Blockscout also reports shows the RPC figure, and an untracked one is admitted as before", async () => { + holdings[TOKEN] = { [FIRST]: 2000000n }; + explorerRows = [ + explorerRow(TOKEN, "TRK", 9000000), + explorerRow(USDC, "USDC", 5000000), + // Too few holders, and neither on the bundled list nor tracked. + explorerRow(OTHER, "SPAM", 5000000, "10"), + // A known symbol from a contract it does not belong to. + explorerRow(OTHER, "USDC", 5000000), + ]; + const [first] = await refresh( + [{ address: FIRST, tokenBalances: [] }], + [TRACKED], + ); + expect(first.tokenBalances).toEqual([ + trackedRow("2.0"), + { + address: USDC, + name: "USDC", + symbol: "USDC", + decimals: 6, + balance: "5.0", + holders: 50000, + }, + ]); +}); + +test("a failed balanceOf leaves the token's previous figure, and is logged", async () => { + explorerRows = []; + const before = trackedRow("7.0"); + const [first] = await refresh( + [{ address: FIRST, tokenBalances: [before] }], + [TRACKED], + ); + expect(first.tokenBalances).toEqual([before]); + expect(logged.some((line) => line.includes("token balance failed"))).toBe( + true, + ); +}); + +test("a balance of zero read through the RPC endpoint replaces the previous figure", async () => { + holdings[TOKEN] = { [FIRST]: 0n }; + const [first] = await refresh( + [{ address: FIRST, tokenBalances: [trackedRow("7.0")] }], + [TRACKED], + ); + expect(first.tokenBalances).toEqual([]); +}); + +test("a tracked token whose scale nothing knows gets no quantity, not a zero", async () => { + holdings[TOKEN] = { [FIRST]: 5n }; + const [first] = await refresh( + [{ address: FIRST, tokenBalances: [] }], + [{ ...TRACKED, decimals: null }], + ); + expect(first.tokenBalances).toEqual([trackedRow(null)]); +}); + +test("a tracked token wearing a known symbol from another contract is not listed", async () => { + holdings[OTHER] = { [FIRST]: 5000000n }; + const [first] = await refresh( + [{ address: FIRST, tokenBalances: [] }], + [{ address: OTHER, symbol: "USDC", name: "Fake", decimals: 6 }], + ); + expect(first.tokenBalances).toEqual([]); +});