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. The entry takes its holder count and
decimals from Blockscout's entry for the token in the same refresh, or
keeps them from before while Blockscout fails. A failed balanceOf keeps
the token's previous entry, or takes Blockscout's entry when there is
none, and is logged. Blockscout's answer still adds the tokens the user
has not tracked, as before.

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 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.
+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.
 
 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,14 +1155,13 @@ 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, 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.
+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.
 
 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
@@ -1382,29 +1381,30 @@ 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; 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
+network is read with `balanceOf` through the RPC endpoint, for every address.
+Its holder count and `decimals` are those of Blockscout's entry for it in the
+same refresh, kept from before while Blockscout fails. A read that fails keeps
+that token's previous entry, as a failed ETH balance keeps its figure, or takes
+Blockscout's entry from the same refresh when there is none. 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`). 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.
@@ -1788,8 +1788,7 @@ 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, the token is tracked, or the token is not in the
-      balance list
+      report a readable one, 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
@@ -2689,10 +2688,9 @@ 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), 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).
+  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).
     - **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.
@@ -2999,10 +2997,8 @@ 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, 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.
+  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.
 
 - **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 via Blockscout, ENS reverse
-// lookup via RPC.
+// tracks via RPC, the other ERC-20 token balances and the holder counts via
+// Blockscout, ENS reverse 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. refreshBalances()
-// keeps only the rows for tokens the user has not tracked: a tracked token's
-// balance is read through the RPC endpoint (fetchTrackedTokenBalances()).
+// explicitly tracked by the user, or have >= 1000 holders. A tracked token's
+// balance is read through the RPC endpoint instead, and its row here gives that
+// entry its holder count and `decimals` (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,21 +208,24 @@ async function fetchTokenBalances(
     }
 }
 
-// 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.
+// The entries at `address` of the tokens the user tracks, 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
+// The balance is read with balanceOf through the RPC endpoint. The name and
+// symbol are the tracked token's. `decimals` and `holders` are those of
+// Blockscout's entry for the token in `explorerRequest`, this refresh's
+// fetchTokenBalances() for the address; while Blockscout fails they are kept
+// from the token's entry in `previous`, the address's entries before this
+// refresh. The balance is stated at the scale the screens read the 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.
+// records, then Blockscout's, as in fetchTokenBalances().
+//
+// 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(
@@ -230,20 +233,22 @@ async function fetchTrackedTokenBalances(
     address,
     trackedTokens,
     previous,
+    explorerRequest,
     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;
+    const listed = trackedTokens.filter(
+        (token) => !isSpoofedSymbol(token.symbol, token.address),
+    );
+    // Each token's balanceOf answer, or null where it failed.
+    const balances = await Promise.all(
+        listed.map(async (token) => {
             try {
                 const contract = new Contract(
                     token.address,
                     ERC20_ABI,
                     provider,
                 );
-                raw = await contract.balanceOf(address);
+                return await contract.balanceOf(address);
             } catch (e) {
                 if (!signal?.aborted) {
                     log.errorf(
@@ -253,27 +258,41 @@ async function fetchTrackedTokenBalances(
                         e.shortMessage || e.message,
                     );
                 }
-                return (
-                    previous.find(
-                        (row) => String(row.address).toLowerCase() === lower,
-                    ) || null
-                );
+                return 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);
+    const explorerRows = await explorerRequest;
+    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.
@@ -341,10 +360,18 @@ async function refreshBalances(
             );
 
             // ERC-20 token balances: the tracked tokens' through the RPC
-            // endpoint, and from Blockscout only the tokens the user has not
+            // endpoint, and from Blockscout the tokens the user has not
             // tracked. When Blockscout fails, the untracked rows from before
-            // this refresh stay.
+            // this refresh stay. The Blockscout request is started first and
+            // 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(
@@ -352,14 +379,10 @@ async function refreshBalances(
                         addr.address,
                         tracked,
                         previous,
+                        explorerRequest,
                         signal,
                     ),
-                    fetchTokenBalances(
-                        addr.address,
-                        blockscoutUrl,
-                        tracked,
-                        signal,
-                    ),
+                    explorerRequest,
                 ]).then(([trackedRows, explorerRows]) => {
                     const untrackedRows = (explorerRows ?? previous).filter(
                         (row) =>

tests/trackedTokenBalances.test.js

@@ -1,6 +1,7 @@
 // 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).
+// RPC endpoint, for every address. Blockscout's answer gives a tracked token its
+// holder count and decimals, and adds 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
@@ -113,15 +114,16 @@ function explorerRow(address, symbol, value, holders = "50000") {
     };
 }
 
-// The row refreshBalances() writes for TOKEN from a balanceOf answer.
-function trackedRow(balance) {
+// The row refreshBalances() writes for TOKEN from a balanceOf answer, with the
+// decimals and holder count Blockscout reported for it, if any.
+function trackedRow(balance, decimals = null, holders = null) {
     return {
         address: TOKEN,
         name: "Tracked Token",
         symbol: "TRK",
-        decimals: null,
+        decimals: decimals,
         balance: balance,
-        holders: null,
+        holders: holders,
     };
 }
 
@@ -183,7 +185,7 @@ test("a tracked token Blockscout also reports shows the RPC figure, and an untra
         [TRACKED],
     );
     expect(first.tokenBalances).toEqual([
-        trackedRow("2.0"),
+        trackedRow("2.0", 6, 50000),
         {
             address: USDC,
             name: "USDC",
@@ -195,9 +197,30 @@ test("a tracked token Blockscout also reports shows the RPC figure, and an untra
     ]);
 });
 
-test("a failed balanceOf leaves the token's previous figure, and is logged", async () => {
-    explorerRows = [];
-    const before = trackedRow("7.0");
+test("a tracked token's entry carries the holder count and decimals Blockscout reports for it", async () => {
+    holdings[TOKEN] = { [FIRST]: 2000000n };
+    // Fewer holders than "Hide tokens with fewer than 1,000 holders" lets
+    // 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],
@@ -208,6 +231,31 @@ test("a failed balanceOf leaves the token's previous figure, and is logged", asy
     );
 });
 
+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(
@@ -226,10 +274,29 @@ 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 wearing a known symbol from another contract is not listed", async () => {
-    holdings[OTHER] = { [FIRST]: 5000000n };
+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: OTHER, symbol: "USDC", name: "Fake", decimals: 6 }],
     );
     expect(first.tokenBalances).toEqual([]);