fix: tracked tokens' balances are read through the RPC endpoint (closes #669)

refreshBalances() now reads the balance of every token tracked on the
active network, at every address, with balanceOf through the same
provider as the ETH balance, at the scale the screens read that token
at. Blockscout's answer adds only the tokens the user has not tracked,
admitted as before. A failed balanceOf keeps that token's previous row
and is logged; while Blockscout fails, the untracked rows from before
stay. A tracked token's row carries no decimals or holder count from
the explorer.

The browser test for a hostile symbol now untracks the stub token
first, because a tracked token is listed under the symbol it was added
with.

Model: opus-5-5

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
+information, and a Blockscout block-explorer API for transaction history and the
-token balances. It also performs best-effort Etherscan address label lookups
+balances of tokens the user has not tracked. It also performs best-effort
-during transaction confirmation. A community-maintained phishing domain
+Etherscan address label lookups during transaction confirmation. A
-blocklist is built into the extension at build time and checked locally; nothing
+community-maintained phishing domain blocklist is built into the extension at
-is fetched for it at runtime.
+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
@@ -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.
+else, and `null` in the entry of a tracked token, whose balance is read through
-Anything that needs the second one calls `resolveTokenDecimals()` — the balance
+the RPC endpoint and not asked of the explorer. It is not the same question as
-list, the approval and swap lines, and the Send screen, which carries the
+the scale a screen should render at. Anything that needs the second one calls
-resolved scale onto the pending transaction for `transferAmount.js` to encode
+`resolveTokenDecimals()` — the balance list, the approval and swap lines, and
-and compare against. Reading the stored field directly instead answers `null`
+the Send screen, which carries the resolved scale onto the pending transaction
-for a bundled or tracked token the explorer merely omitted, which is not a
+for `transferAmount.js` to encode and compare against. Reading the stored field
-refusal the wallet has any reason to make.
+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
@@ -1381,30 +1382,29 @@ The core hierarchy is **Wallets → Addresses**:
 
 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.
+network is read with `balanceOf` through the RPC endpoint, for every address; a
-Its holder count and `decimals` are those of Blockscout's entry for it in the
+read that fails leaves that token's previous figure, as a failed ETH balance
-same refresh, kept from before while Blockscout fails. A read that fails keeps
+does. Every other token comes from the Blockscout `token-balances` response
-that token's previous entry, as a failed ETH balance keeps its figure, or takes
+(`fetchTokenBalances()`), so tokens do appear without the user adding them, and
-Blockscout's entry from the same refresh when there is none. Every other token
+they keep their previous figures while Blockscout fails. A tracked token is
-comes from the Blockscout `token-balances` response (`fetchTokenBalances()`), so
+shown when its balance is nonzero, and any other ERC-20 when its balance is
-tokens do appear without the user adding them, and they keep their previous
+nonzero and it is in the bundled known-token list or has 1,000 or more holders.
-figures while Blockscout fails. A tracked token is shown when its balance is
+A token claiming a symbol from the bundled list from any other contract address
-nonzero, and any other ERC-20 when its balance is nonzero and it is in the
+is always dropped, tracked or not, and so is any token claiming a symbol that
-bundled known-token list or has 1,000 or more holders. A token claiming a symbol
+belongs to the native asset and therefore has no legitimate contract at all
-from the bundled list from any other contract address is always dropped, tracked
+(`"ETH"`, and every network's `nativeCurrency`, such as `"SepoliaETH"`, on every
-or not, and so is any token claiming a symbol that belongs to the native asset
+network). That filter is unconditional — the "Hide tokens with fewer than 1,000
-and therefore has no legitimate contract at all (`"ETH"`, and every network's
+holders" setting governs the transaction history and the send-screen token
-`nativeCurrency`, such as `"SepoliaETH"`, on every network). That filter is
+selector, not this list. A token's holder count is unknown when the explorer
-unconditional — the "Hide tokens with fewer than 1,000 holders" setting governs
+reports none, or reports anything other than a whole number written in digits
-the transaction history and the send-screen token selector, not this list. A
+alone, such as `1,000` or `1e3` (`parseHoldersCount()` in
-token's holder count is unknown when the explorer reports none, or reports
+`src/shared/holders.js`); a tracked token's entry has none, because the explorer
-anything other than a whole number written in digits alone, such as `1,000` or
+is not asked about it. This list does not take an unknown count as 1,000 or
-`1e3` (`parseHoldersCount()` in `src/shared/holders.js`). This list does not
+more, so such a token is shown only when it is on the bundled list or tracked.
-take an unknown count as 1,000 or more, so such a token is shown only when it is
+Every nonzero holding of a token shown is stored exactly, however small, but a
-on the bundled list or tracked. Every nonzero holding of a token shown is stored
+holding below 0.000001 is left out of the balance lists, the send-screen token
-exactly, however small, but a holding below 0.000001 is left out of the balance
+selector, the address total and the remove-address warning
-lists, the send-screen token selector, the address total and the remove-address
+(`isBelowOneMillionth()` in `src/shared/amountDisplay.js`). The Send and
-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.
@@ -1788,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
@@ -2688,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
+  transactions and ERC-20 token transfers), the balances of ERC-20 tokens the
-  holder counts (for spam filtering). The default endpoint is
+  user has not tracked, and token holder counts (for spam filtering). The
-  `https://eth.blockscout.com/api/v2` (configurable by the user in Settings).
+  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.
@@ -2997,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
+  the selector, and so is every token the user tracks, whose entry carries no
-  token is one on the bundled list or one the user tracks.
+  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

src/shared/balances.js

@@ -1,6 +1,6 @@
 // Balance fetching: ETH balances and the balances of the tokens the user
-// tracks via RPC, the other ERC-20 token balances and the holder counts via
+// tracks via RPC, the other ERC-20 token balances via Blockscout, ENS reverse
-// Blockscout, ENS reverse lookup via RPC.
+// lookup via RPC.
 
 const {
     JsonRpcProvider,
@@ -82,9 +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. A tracked token's
+// explicitly tracked by the user, or have >= 1000 holders. refreshBalances()
-// balance is read through the RPC endpoint instead, and its row here gives that
+// keeps only the rows for tokens the user has not tracked: a tracked token's
-// entry its holder count and `decimals` (fetchTrackedTokenBalances()).
+// 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
@@ -208,24 +208,21 @@ async function fetchTokenBalances(
     }
 }
 
-// The entries at `address` of the tokens the user tracks, shaped as
+// The balance at `address` of each token the user tracks, read with balanceOf
-// fetchTokenBalances() returns them, for the tokens the address holds.
+// through the RPC endpoint. Returns rows shaped as fetchTokenBalances() returns
+// them, for the tokens the address holds.
 //
-// The balance is read with balanceOf through the RPC endpoint. The name and
+// A token whose balanceOf fails keeps its row from `previous`, the address's
-// symbol are the tracked token's. `decimals` and `holders` are those of
+// rows before this refresh, as a failed ETH balance keeps its figure, and the
-// Blockscout's entry for the token in `explorerRequest`, this refresh's
+// failure is logged. A balance of zero gets no row, as a zero Blockscout
-// fetchTokenBalances() for the address; while Blockscout fails they are kept
+// reports gets none: the balance lists take a tracked token's zero from the
-// from the token's entry in `previous`, the address's entries before this
+// tracked list. A token whose symbol the spoof rule refuses gets no row either,
-// refresh. The balance is stated at the scale the screens read the token at
+// 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, then Blockscout's, as in fetchTokenBalances().
+// records. `decimals` and `holders` hold what Blockscout reported for a token;
-//
+// Blockscout is not asked about a tracked token, so both are null.
-// A token whose balanceOf fails keeps its entry in `previous`, as a failed ETH
-// balance keeps its figure, or takes Blockscout's entry from this refresh when
-// it has none. The failure is logged. A balance of zero gets no entry, 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 entry either, as in fetchTokenBalances().
 //
 // `signal` is as for fetchTokenBalances().
 async function fetchTrackedTokenBalances(
@@ -233,22 +230,20 @@ async function fetchTrackedTokenBalances(
     address,
     trackedTokens,
     previous,
-    explorerRequest,
     signal,
 ) {
-    const listed = trackedTokens.filter(
+    const rows = await Promise.all(
-        (token) => !isSpoofedSymbol(token.symbol, token.address),
+        trackedTokens.map(async (token) => {
-    );
+            if (isSpoofedSymbol(token.symbol, token.address)) return null;
-    // Each token's balanceOf answer, or null where it failed.
+            const lower = token.address.toLowerCase();
-    const balances = await Promise.all(
+            let raw;
-        listed.map(async (token) => {
             try {
                 const contract = new Contract(
                     token.address,
                     ERC20_ABI,
                     provider,
                 );
-                return await contract.balanceOf(address);
+                raw = await contract.balanceOf(address);
             } catch (e) {
                 if (!signal?.aborted) {
                     log.errorf(
@@ -258,41 +253,27 @@ async function fetchTrackedTokenBalances(
                         e.shortMessage || e.message,
                     );
                 }
-                return null;
+                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,
+            };
         }),
     );
-    const explorerRows = await explorerRequest;
+    return rows.filter((row) => row !== null);
-    const reported = explorerRows ?? previous;
-
-    const rows = [];
-    listed.forEach((token, i) => {
-        const lower = token.address.toLowerCase();
-        const entryIn = (list) =>
-            list.find((row) => String(row.address).toLowerCase() === lower);
-        const raw = balances[i];
-        if (raw === null) {
-            const kept = entryIn(previous) || entryIn(explorerRows || []);
-            if (kept) rows.push(kept);
-            return;
-        }
-        if (raw === 0n) return;
-        const entry = entryIn(reported);
-        const decimals = entry ? entry.decimals : null;
-        const known = resolveTokenDecimals(lower, { trackedTokens });
-        const scale = known !== null ? known : decimals;
-        rows.push({
-            address: token.address,
-            name: token.name || "",
-            symbol: token.symbol || "???",
-            decimals: decimals,
-            // null when nothing knows the scale: a holding whose amount
-            // cannot be stated, never a zero.
-            balance: scale === null ? null : formatTokenBalance(raw, scale),
-            holders: entry ? entry.holders : null,
-        });
-    });
-    return rows;
 }
 
 // Fetch ETH balances, ENS names, and ERC-20 token balances for all addresses.
@@ -360,18 +341,10 @@ async function refreshBalances(
             );
 
             // ERC-20 token balances: the tracked tokens' through the RPC
-            // endpoint, and from Blockscout the tokens the user has not
+            // endpoint, and from Blockscout only the tokens the user has not
             // tracked. When Blockscout fails, the untracked rows from before
-            // this refresh stay. The Blockscout request is started first and
+            // this refresh stay.
-            // handed to fetchTrackedTokenBalances() too, which waits for it
-            // after sending its balanceOf reads, so the two run side by side.
             const previous = addr.tokenBalances || [];
-            const explorerRequest = fetchTokenBalances(
-                addr.address,
-                blockscoutUrl,
-                tracked,
-                signal,
-            );
             updates.push(
                 Promise.all([
                     fetchTrackedTokenBalances(
@@ -379,10 +352,14 @@ async function refreshBalances(
                         addr.address,
                         tracked,
                         previous,
-                        explorerRequest,
                         signal,
                     ),
-                    explorerRequest,
+                    fetchTokenBalances(
+                        addr.address,
+                        blockscoutUrl,
+                        tracked,
+                        signal,
+                    ),
                 ]).then(([trackedRows, explorerRows]) => {
                     const untrackedRows = (explorerRows ?? previous).filter(
                         (row) =>

tests/trackedTokenBalances.test.js

@@ -1,7 +1,6 @@
 // The balance of each token the user tracks is read with balanceOf through the
-// RPC endpoint, for every address. Blockscout's answer gives a tracked token its
+// RPC endpoint, for every address, and Blockscout's answer adds only the tokens
-// holder count and decimals, and adds the tokens the user has not tracked
+// the user has not tracked (https://git.eeqj.de/sneak/AutistMask/issues/669).
-// (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
@@ -114,16 +113,15 @@ function explorerRow(address, symbol, value, holders = "50000") {
     };
 }
 
-// The row refreshBalances() writes for TOKEN from a balanceOf answer, with the
+// The row refreshBalances() writes for TOKEN from a balanceOf answer.
-// decimals and holder count Blockscout reported for it, if any.
+function trackedRow(balance) {
-function trackedRow(balance, decimals = null, holders = null) {
     return {
         address: TOKEN,
         name: "Tracked Token",
         symbol: "TRK",
-        decimals: decimals,
+        decimals: null,
         balance: balance,
-        holders: holders,
+        holders: null,
     };
 }
 
@@ -185,7 +183,7 @@ test("a tracked token Blockscout also reports shows the RPC figure, and an untra
         [TRACKED],
     );
     expect(first.tokenBalances).toEqual([
-        trackedRow("2.0", 6, 50000),
+        trackedRow("2.0"),
         {
             address: USDC,
             name: "USDC",
@@ -197,30 +195,9 @@ test("a tracked token Blockscout also reports shows the RPC figure, and an untra
     ]);
 });
 
-test("a tracked token's entry carries the holder count and decimals Blockscout reports for it", async () => {
+test("a failed balanceOf leaves the token's previous figure, and is logged", async () => {
-    holdings[TOKEN] = { [FIRST]: 2000000n };
+    explorerRows = [];
-    // Fewer holders than "Hide tokens with fewer than 1,000 holders" lets
+    const before = trackedRow("7.0");
-    // through to Send's token list.
-    explorerRows = [explorerRow(TOKEN, "TRK", 9000000, "10")];
-    const [first] = await refresh(
-        [{ address: FIRST, tokenBalances: [] }],
-        [TRACKED],
-    );
-    expect(first.tokenBalances).toEqual([trackedRow("2.0", 6, 10)]);
-});
-
-test("while Blockscout fails, a tracked token keeps the holder count and decimals of its entry from before", async () => {
-    holdings[TOKEN] = { [FIRST]: 2000000n };
-    const [first] = await refresh(
-        [{ address: FIRST, tokenBalances: [trackedRow("7.0", 6, 10)] }],
-        [TRACKED],
-    );
-    expect(first.tokenBalances).toEqual([trackedRow("2.0", 6, 10)]);
-});
-
-test("a failed balanceOf keeps the token's entry from before, and is logged", async () => {
-    explorerRows = [explorerRow(TOKEN, "TRK", 9000000)];
-    const before = trackedRow("7.0", 6, 50000);
     const [first] = await refresh(
         [{ address: FIRST, tokenBalances: [before] }],
         [TRACKED],
@@ -231,31 +208,6 @@ test("a failed balanceOf keeps the token's entry from before, and is logged", as
     );
 });
 
-test("a failed balanceOf with no entry from before takes Blockscout's entry from this refresh", async () => {
-    // balanceOf fails at FIRST and answers at SECOND.
-    holdings[TOKEN] = { [SECOND]: 2000000n };
-    explorerRows = [explorerRow(TOKEN, "TRK", 9000000)];
-    const [first, second] = await refresh(
-        [
-            { address: FIRST, tokenBalances: [] },
-            { address: SECOND, tokenBalances: [] },
-        ],
-        [TRACKED],
-    );
-    expect(first.tokenBalances).toEqual([
-        {
-            address: TOKEN,
-            name: "TRK",
-            symbol: "TRK",
-            decimals: 6,
-            balance: "9.0",
-            holders: 50000,
-        },
-    ]);
-    // Where balanceOf answers, its figure stands.
-    expect(second.tokenBalances).toEqual([trackedRow("2.0", 6, 50000)]);
-});
-
 test("a balance of zero read through the RPC endpoint replaces the previous figure", async () => {
     holdings[TOKEN] = { [FIRST]: 0n };
     const [first] = await refresh(
@@ -274,29 +226,10 @@ test("a tracked token whose scale nothing knows gets no quantity, not a zero", a
     expect(first.tokenBalances).toEqual([trackedRow(null)]);
 });
 
-test("a tracked token whose scale only Blockscout reports is stated at that scale", async () => {
-    holdings[TOKEN] = { [FIRST]: 2000000n };
-    explorerRows = [explorerRow(TOKEN, "TRK", 9000000)];
-    const [first] = await refresh(
-        [{ address: FIRST, tokenBalances: [] }],
-        [{ ...TRACKED, decimals: null }],
-    );
-    expect(first.tokenBalances).toEqual([trackedRow("2.0", 6, 50000)]);
-});
-
 test("a tracked token wearing a known symbol from another contract is not listed", async () => {
     holdings[OTHER] = { [FIRST]: 5000000n };
-    // Its entry from before goes too, although Blockscout fails.
-    const before = {
-        address: OTHER,
-        name: "Fake",
-        symbol: "USDC",
-        decimals: 6,
-        balance: "5.0",
-        holders: 50000,
-    };
     const [first] = await refresh(
-        [{ address: FIRST, tokenBalances: [before] }],
+        [{ address: FIRST, tokenBalances: [] }],
         [{ address: OTHER, symbol: "USDC", name: "Fake", decimals: 6 }],
     );
     expect(first.tokenBalances).toEqual([]);