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
+information, and a Blockscout block-explorer API for transaction history and
-balances of tokens the user has not tracked. It also performs best-effort
+token balances. It also performs best-effort Etherscan address label lookups
-Etherscan address label lookups during transaction confirmation. A
+during transaction confirmation. A community-maintained phishing domain
-community-maintained phishing domain blocklist is built into the extension at
+blocklist is built into the extension at build time and checked locally; nothing
-build time and checked locally; nothing is fetched for it at runtime.
+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
+else, which is not the same question as the scale a screen should render at.
-the RPC endpoint and not asked of the explorer. It is not the same question as
+Anything that needs the second one calls `resolveTokenDecimals()` — the balance
-the scale a screen should render at. Anything that needs the second one calls
+list, the approval and swap lines, and the Send screen, which carries the
-`resolveTokenDecimals()` — the balance list, the approval and swap lines, and
+resolved scale onto the pending transaction for `transferAmount.js` to encode
-the Send screen, which carries the resolved scale onto the pending transaction
+and compare against. Reading the stored field directly instead answers `null`
-for `transferAmount.js` to encode and compare against. Reading the stored field
+for a bundled or tracked token the explorer merely omitted, which is not a
-directly instead answers `null` for a bundled or tracked token the explorer
+refusal the wallet has any reason to make.
-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
+network is read with `balanceOf` through the RPC endpoint, for every address.
-read that fails leaves that token's previous figure, as a failed ETH balance
+Its holder count and `decimals` are those of Blockscout's entry for it in the
-does. Every other token comes from the Blockscout `token-balances` response
+same refresh, kept from before while Blockscout fails. A read that fails keeps
-(`fetchTokenBalances()`), so tokens do appear without the user adding them, and
+that token's previous entry, as a failed ETH balance keeps its figure, or takes
-they keep their previous figures while Blockscout fails. A tracked token is
+Blockscout's entry from the same refresh when there is none. Every other token
-shown when its balance is nonzero, and any other ERC-20 when its balance is
+comes from the Blockscout `token-balances` response (`fetchTokenBalances()`), so
-nonzero and it is in the bundled known-token list or has 1,000 or more holders.
+tokens do appear without the user adding them, and they keep their previous
-A token claiming a symbol from the bundled list from any other contract address
+figures while Blockscout fails. A tracked token is shown when its balance is
-is always dropped, tracked or not, and so is any token claiming a symbol that
+nonzero, and any other ERC-20 when its balance is nonzero and it is in the
-belongs to the native asset and therefore has no legitimate contract at all
+bundled known-token list or has 1,000 or more holders. A token claiming a symbol
-(`"ETH"`, and every network's `nativeCurrency`, such as `"SepoliaETH"`, on every
+from the bundled list from any other contract address is always dropped, tracked
-network). That filter is unconditional — the "Hide tokens with fewer than 1,000
+or not, and so is any token claiming a symbol that belongs to the native asset
-holders" setting governs the transaction history and the send-screen token
+and therefore has no legitimate contract at all (`"ETH"`, and every network's
-selector, not this list. A token's holder count is unknown when the explorer
+`nativeCurrency`, such as `"SepoliaETH"`, on every network). That filter is
-reports none, or reports anything other than a whole number written in digits
+unconditional — the "Hide tokens with fewer than 1,000 holders" setting governs
-alone, such as `1,000` or `1e3` (`parseHoldersCount()` in
+the transaction history and the send-screen token selector, not this list. A
-`src/shared/holders.js`); a tracked token's entry has none, because the explorer
+token's holder count is unknown when the explorer reports none, or reports
-is not asked about it. This list does not take an unknown count as 1,000 or
+anything other than a whole number written in digits alone, such as `1,000` or
-more, so such a token is shown only when it is on the bundled list or tracked.
+`1e3` (`parseHoldersCount()` in `src/shared/holders.js`). This list does not
-Every nonzero holding of a token shown is stored exactly, however small, but a
+take an unknown count as 1,000 or more, so such a token is shown only when it is
-holding below 0.000001 is left out of the balance lists, the send-screen token
+on the bundled list or tracked. Every nonzero holding of a token shown is stored
-selector, the address total and the remove-address warning
+exactly, however small, but a holding below 0.000001 is left out of the balance
-(`isBelowOneMillionth()` in `src/shared/amountDisplay.js`). The Send and
+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
+      report a readable one, or the token is not in the balance list
-      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
+  transactions and ERC-20 token transfers), ERC-20 token balances, and token
-  user has not tracked, and token holder counts (for spam filtering). The
+  holder counts (for spam filtering). The default endpoint is
-  default endpoint is `https://eth.blockscout.com/api/v2` (configurable by the
+  `https://eth.blockscout.com/api/v2` (configurable by the user in Settings).
-  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
+  the selector. The selector offers only tokens in the balance list, so such a
-  holder count (see Data Model). The selector offers only tokens in the balance
+  token is one on the bundled list or one the user tracks.
-  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 via Blockscout, ENS reverse
+// tracks via RPC, the other ERC-20 token balances and the holder counts via
-// lookup via RPC.
+// 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()
+// explicitly tracked by the user, or have >= 1000 holders. A tracked token's
-// keeps only the rows for tokens the user has not tracked: a tracked token's
+// balance is read through the RPC endpoint instead, and its row here gives that
-// balance is read through the RPC endpoint (fetchTrackedTokenBalances()).
+// 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
+// The entries at `address` of the tokens the user tracks, shaped as
-// through the RPC endpoint. Returns rows shaped as fetchTokenBalances() returns
+// fetchTokenBalances() returns them, for the tokens the address holds.
-// them, for the tokens the address holds.
 //
-// A token whose balanceOf fails keeps its row from `previous`, the address's
+// The balance is read with balanceOf through the RPC endpoint. The name and
-// rows before this refresh, as a failed ETH balance keeps its figure, and the
+// symbol are the tracked token's. `decimals` and `holders` are those of
-// failure is logged. A balance of zero gets no row, as a zero Blockscout
+// Blockscout's entry for the token in `explorerRequest`, this refresh's
-// reports gets none: the balance lists take a tracked token's zero from the
+// fetchTokenBalances() for the address; while Blockscout fails they are kept
-// tracked list. A token whose symbol the spoof rule refuses gets no row either,
+// from the token's entry in `previous`, the address's entries before this
-// as in fetchTokenBalances().
+// refresh. The balance is stated at the scale the screens read the token at
-//
-// 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;
+// records, then Blockscout's, as in fetchTokenBalances().
-// 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(
@@ -230,20 +233,22 @@ async function fetchTrackedTokenBalances(
     address,
     trackedTokens,
     previous,
+    explorerRequest,
     signal,
 ) {
-    const rows = await Promise.all(
+    const listed = trackedTokens.filter(
-        trackedTokens.map(async (token) => {
+        (token) => !isSpoofedSymbol(token.symbol, token.address),
-            if (isSpoofedSymbol(token.symbol, token.address)) return null;
+    );
-            const lower = token.address.toLowerCase();
+    // Each token's balanceOf answer, or null where it failed.
-            let raw;
+    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 (
+                return null;
-                    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);
+    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(
+                    explorerRequest,
-                        addr.address,
-                        blockscoutUrl,
-                        tracked,
-                        signal,
-                    ),
                 ]).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
+// RPC endpoint, for every address. Blockscout's answer gives a tracked token its
-// the user has not tracked (https://git.eeqj.de/sneak/AutistMask/issues/669).
+// 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.
+// The row refreshBalances() writes for TOKEN from a balanceOf answer, with the
-function trackedRow(balance) {
+// 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 () => {
+test("a tracked token's entry carries the holder count and decimals Blockscout reports for it", async () => {
-    explorerRows = [];
+    holdings[TOKEN] = { [FIRST]: 2000000n };
-    const before = trackedRow("7.0");
+    // 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 () => {
+test("a tracked token whose scale only Blockscout reports is stated at that scale", async () => {
-    holdings[OTHER] = { [FIRST]: 5000000n };
+    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([]);