diff --git a/README.md b/README.md index 278c5a6..2515733 100644 --- a/README.md +++ b/README.md @@ -695,6 +695,7 @@ src/ balances.js — ETH + ERC-20 balance fetching via RPC + 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 prices.js — ETH/USD and token/USD via CoinDesk API scamlist.js — known fraud contract addresses state.js — persisted state (extension storage) @@ -1123,13 +1124,18 @@ 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. `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 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. +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 +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. #### Stored state and its version @@ -1438,7 +1444,9 @@ view would leave a wallet one click from deletion. - Send / Receive buttons - Token contract well (ERC-20 only): full contract address (tap to copy, etherscan link) plus name, symbol, decimals, holder count and project - website where known + 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 - Token-filtered transaction list (only this token's transfers) - **Transitions**: - "Send" → **Send** (token locked: the dropdown is replaced by a static @@ -2389,7 +2397,8 @@ indexes it as a real token transfer. fewer than 1,000 holders are hidden from transaction history by default. Legitimate tokens have substantial holder counts; poisoning tokens typically have zero. This catches new poisoning contracts that use novel symbols not in - the known token list. + the known token list. A transfer whose token's holder count is unknown (see + Data Model) is kept: only a reported count below 1,000 hides it. - **Fraud contract blocklist**: AutistMask maintains a local list of known fraud contract addresses. Token transfers involving these contracts are filtered @@ -2399,7 +2408,9 @@ indexes it as a real token transfer. - **Send-side token filtering**: Tokens with fewer than 1,000 holders are 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. + 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. - **Dust transaction filtering**: A second wave of the same attack used real native ETH transfers instead of fake tokens. Transaction @@ -2423,8 +2434,9 @@ indexes it as a real token transfer. both cases identically to the history. The fraud contract blocklist is applied unconditionally on that selector and is not consulted by the balance list at all. The low-holder setting also gates the send selector, while the balance - list's own 1,000-holder floor is unconditional (see Data Model). The dust - threshold applies to the transaction history alone. + list's own 1,000-holder floor is unconditional (see Data Model). An unknown + holder count passes the history and send-selector filters but not that floor. + The dust threshold applies to the transaction history alone. #### Phishing Domain Protection diff --git a/TODO.md b/TODO.md index 0de9823..fa4acd0 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,17 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-05: A `holders_count` that is not a whole number in plain digits is + unknown, not read in part + ([#251](https://git.eeqj.de/sneak/AutistMask/issues/251)). `parseInt` read + `1,000` as 1, `0x10` as 0 and `1e3` as 1, a reported low count that hides the + token in the transaction history and the send-screen token selector. A count + above `Number.MAX_SAFE_INTEGER` is unknown too, not rounded or `Infinity`. The + balance list's `holders !== null` check, which did nothing, is dropped. + `README.md` and `docs/README.md` now say how each filter treats an unknown + count and that the token screen leaves out its "Holders:" row then, and + `README.md` lists `src/shared/holders.js`. + - 2026-10-04: A popup boot in the tests loads transactions without failing ([#429](https://git.eeqj.de/sneak/AutistMask/issues/429)). The stand-in for `filterTransactions` in `tests/support/popupBoot.js` returned a bare list, diff --git a/docs/README.md b/docs/README.md index deac2a3..032fca3 100644 --- a/docs/README.md +++ b/docs/README.md @@ -333,7 +333,13 @@ it is hidden from your transaction history and from the send token list. from transaction history and the send token list, and are left out of your balances unless they are on the bundled known-token list or you added them yourself. Legitimate tokens have substantial holder counts; scam tokens deployed -for address poisoning typically have zero. +for address poisoning typically have zero. When the explorer reports no holder +count for a token, or reports something other than a whole number in plain +digits (such as "1,000"), the count is unknown. An unknown count does not hide a +token from your transaction history or the send token list, and it does not get +a token into your balances either: such a token is listed only if it is on the +bundled known-token list or you added it yourself. The screen you reach by +clicking a token balance shows a "Holders:" line only when the count is known. **Fraud contract blocklist.** When AutistMask detects a fraudulent transfer, it adds the contract address to a local blocklist. Future transactions from that diff --git a/src/shared/balances.js b/src/shared/balances.js index dcbbabf..81a8d09 100644 --- a/src/shared/balances.js +++ b/src/shared/balances.js @@ -147,20 +147,20 @@ async function fetchTokenBalances(address, blockscoutUrl, trackedTokens) { // null is a holding of an amount that cannot be stated, which is // not the same as a holding of zero, and must never render as one. const bal = scale === null ? null : formatTokenBalance(raw, scale); - // null means the explorer reported no count, which is not the - // same as a count of zero. This gate is not the low-holder - // display filter: it has no user-facing off switch and governs - // the whole balance list, so it stays strict and admits a token - // only on a reported count — an unreported one is no evidence. - // A legitimate token still reaches the list through the known + // null means the explorer reported no readable count, which is + // not the same as a count of zero. This gate is not the + // low-holder display filter: it has no user-facing off switch and + // governs the whole balance list, so it stays strict and admits a + // token only on a reported count — an unreported one is no + // evidence, and `null >= LOW_HOLDER_THRESHOLD` is false. A + // legitimate token still reaches the list through the known // token list or by the user tracking it, and the null is carried // through to the views, where the two low-holder filters treat // an unknown count as "do not judge" rather than as zero. const holders = parseHoldersCount(item.token.holders_count); const isKnown = TOKEN_BY_ADDRESS.has(tokenAddr); const isTracked = trackedSet.has(tokenAddr); - const hasEnoughHolders = - holders !== null && holders >= LOW_HOLDER_THRESHOLD; + const hasEnoughHolders = holders >= LOW_HOLDER_THRESHOLD; // Skip spam tokens the user never asked to see if (!isKnown && !isTracked && !hasEnoughHolders) continue; diff --git a/src/shared/holders.js b/src/shared/holders.js index 5038be1..a6ea65e 100644 --- a/src/shared/holders.js +++ b/src/shared/holders.js @@ -9,13 +9,22 @@ const LOW_HOLDER_THRESHOLD = 1000; -// Parse an explorer-supplied holders_count into a number, or null when the -// explorer did not report one. Anything unparseable is unknown too: a count -// we cannot read is not a count of zero. +// Parse an explorer-supplied holders_count into a number, or null when it is +// not one. Only a whole number of zero or more, or a string made of nothing +// but the digits 0-9, is a count. Anything else is null, never read in part: +// "1,000", "0x10" and "1e3" are unknown, not 1, 0 and 1, because a count we +// cannot read is not a low count. A count above Number.MAX_SAFE_INTEGER is +// null too: a number cannot hold it exactly, so it would come back rounded, +// or as Infinity. function parseHoldersCount(raw) { - if (raw === null || raw === undefined || raw === "") return null; - const n = parseInt(raw, 10); - return Number.isFinite(n) ? n : null; + if (typeof raw === "number") { + return Number.isSafeInteger(raw) && raw >= 0 ? raw : null; + } + if (typeof raw === "string" && /^[0-9]+$/.test(raw)) { + const count = Number(raw); + return Number.isSafeInteger(count) ? count : null; + } + return null; } // True only for a token the explorer reported as having fewer holders than diff --git a/src/shared/transactions.js b/src/shared/transactions.js index 51702e1..8e8a42b 100644 --- a/src/shared/transactions.js +++ b/src/shared/transactions.js @@ -137,9 +137,10 @@ function parseTokenTransfer(tt, addrLower, chainId) { contractAddress: normalizeAddress( tt.token?.address_hash || tt.token?.address || "", ), - // null when the explorer reported no count: unknown, not zero. The - // low-holder filter declines to judge a null, so a legitimate token - // is not hidden because a field went missing upstream. + // null when the explorer reported no readable count: unknown, not + // zero. The low-holder filter declines to judge a null, so a + // legitimate token is not hidden because a field went missing + // upstream. holders: parseHoldersCount(tt.token?.holders_count), chainId: chainId, }; diff --git a/tests/holders.test.js b/tests/holders.test.js index 48d7ce7..f446aa4 100644 --- a/tests/holders.test.js +++ b/tests/holders.test.js @@ -57,6 +57,39 @@ describe("parseHoldersCount", () => { expect(parseHoldersCount("many")).toBeNull(); expect(parseHoldersCount(NaN)).toBeNull(); }); + + // Each of these starts with a digit, so reading only the leading digits + // would turn it into a small reported count, and a small count is + // exactly what hides a token as spam (issue #251). + test.each(["1,000", "0x10", "1e3", "12 holders"])( + "%p is not read in part: it is unknown", + (raw) => { + expect(parseHoldersCount(raw)).toBeNull(); + }, + ); + + test("a negative count is unknown", () => { + expect(parseHoldersCount("-5")).toBeNull(); + expect(parseHoldersCount(-5)).toBeNull(); + }); + + // A number holds a whole number exactly only up to 2^53 - 1. Past that a + // string of digits would come back rounded, and a long enough one as + // Infinity, which would pass every holder-count floor. + test("a count too large for a number to hold exactly is unknown", () => { + expect(parseHoldersCount("9007199254740993")).toBeNull(); + expect(parseHoldersCount("9".repeat(400))).toBeNull(); + expect(parseHoldersCount(2 ** 53)).toBeNull(); + }); + + test("the largest count a number holds exactly still parses", () => { + expect(parseHoldersCount("9007199254740991")).toBe( + Number.MAX_SAFE_INTEGER, + ); + expect(parseHoldersCount(Number.MAX_SAFE_INTEGER)).toBe( + Number.MAX_SAFE_INTEGER, + ); + }); }); describe("isLowHolderCount", () => {