fix: distinguish an unknown holder count from zero so a legitimate token is not filtered (closes #230)
All checks were successful
check / check (push) Successful in 39s

This commit was merged in pull request #244.
This commit is contained in:
2026-08-12 10:34:45 +02:00
parent bf1dbec87c
commit ce4a0d7b8d
8 changed files with 412 additions and 7 deletions

View File

@@ -44,6 +44,10 @@ undefined identifiers, which is how
# Completed Steps # Completed Steps
- 2026-08-12: An unreported `holders_count` is now parsed as `null` rather than
`0`, so the low-holder rule declines to judge an unknown count instead of
hiding a legitimate token as spam, in both the transaction history and the
Send token selector ([#230](https://git.eeqj.de/sneak/AutistMask/issues/230)).
- 2026-08-12: Bundled token list documentation no longer states a count. The - 2026-08-12: Bundled token list documentation no longer states a count. The
four "top 250" claims in `README.md` and the "roughly 500" claim in four "top 250" claims in `README.md` and the "roughly 500" claim in
`docs/README.md` are replaced with a description of how the list is actually `docs/README.md` are replaced with a description of how the list is actually

View File

@@ -13,6 +13,7 @@ const { state, currentAddress } = require("../../shared/state");
let ctx; let ctx;
const { getProvider } = require("../../shared/balances"); const { getProvider } = require("../../shared/balances");
const { KNOWN_SYMBOLS, resolveSymbol } = require("../../shared/tokenList"); const { KNOWN_SYMBOLS, resolveSymbol } = require("../../shared/tokenList");
const { isLowHolderCount } = require("../../shared/holders");
const { getAddress } = require("ethers"); const { getAddress } = require("ethers");
const ZERO_ADDRESS = "0x0000000000000000000000000000000000000000"; const ZERO_ADDRESS = "0x0000000000000000000000000000000000000000";
@@ -132,7 +133,10 @@ function renderSendTokenSelect(addr) {
for (const t of addr.tokenBalances || []) { for (const t of addr.tokenBalances || []) {
if (isSpoofedToken(t)) continue; if (isSpoofedToken(t)) continue;
if (fraudSet.has(t.address.toLowerCase())) continue; if (fraudSet.has(t.address.toLowerCase())) continue;
if (state.hideLowHolderTokens && (t.holders || 0) < 1000) continue; // An unknown holder count does not withhold a token the user holds:
// only a count the explorer actually reported as below the threshold
// does. Otherwise a missing field makes a real asset unspendable.
if (state.hideLowHolderTokens && isLowHolderCount(t.holders)) continue;
const opt = document.createElement("option"); const opt = document.createElement("option");
opt.value = t.address; opt.value = t.address;
opt.textContent = t.symbol; opt.textContent = t.symbol;

View File

@@ -12,6 +12,7 @@ const { ERC20_ABI } = require("./constants");
const { log, debugFetch } = require("./log"); const { log, debugFetch } = require("./log");
const { deriveAddressFromXpub } = require("./wallet"); const { deriveAddressFromXpub } = require("./wallet");
const { KNOWN_SYMBOLS, TOKEN_BY_ADDRESS } = require("./tokenList"); const { KNOWN_SYMBOLS, TOKEN_BY_ADDRESS } = require("./tokenList");
const { LOW_HOLDER_THRESHOLD, parseHoldersCount } = require("./holders");
// Use a static network to skip auto-detection (which can fail and cause // Use a static network to skip auto-detection (which can fail and cause
// "could not coalesce error" on some RPC endpoints like Cloudflare). // "could not coalesce error" on some RPC endpoints like Cloudflare).
@@ -70,10 +71,20 @@ async function fetchTokenBalances(address, blockscoutUrl, trackedTokens) {
if (bal === "0.0") continue; if (bal === "0.0") continue;
const tokenAddr = (item.token.address_hash || "").toLowerCase(); const tokenAddr = (item.token.address_hash || "").toLowerCase();
const holders = parseInt(item.token.holders_count || "0", 10); // 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
// 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 isKnown = TOKEN_BY_ADDRESS.has(tokenAddr);
const isTracked = trackedSet.has(tokenAddr); const isTracked = trackedSet.has(tokenAddr);
const hasEnoughHolders = holders >= 1000; const hasEnoughHolders =
holders !== null && holders >= LOW_HOLDER_THRESHOLD;
// Skip spam tokens the user never asked to see // Skip spam tokens the user never asked to see
if (!isKnown && !isTracked && !hasEnoughHolders) continue; if (!isKnown && !isTracked && !hasEnoughHolders) continue;
@@ -278,6 +289,7 @@ async function scanForAddresses(xpub, rpcUrl, gapLimit = 5) {
} }
module.exports = { module.exports = {
fetchTokenBalances,
refreshBalances, refreshBalances,
lookupTokenInfo, lookupTokenInfo,
getProvider, getProvider,

32
src/shared/holders.js Normal file
View File

@@ -0,0 +1,32 @@
// Holder counts, and the one rule that decides whether a count is "low".
//
// The block explorer's holders_count is optional: it is absent on a token it
// has only just indexed, and it goes missing on a degraded or changed API.
// Absent means the count is unknown. It does not mean the token has no
// holders, and collapsing the two hides a token the user really holds as if
// it were spam. Every call site reads the count through here so the
// distinction cannot be lost again in one place while holding in the others.
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.
function parseHoldersCount(raw) {
if (raw === null || raw === undefined || raw === "") return null;
const n = parseInt(raw, 10);
return Number.isFinite(n) ? n : null;
}
// True only for a token the explorer reported as having fewer holders than
// the threshold. An unknown count is never low: showing a spam token the
// user can see is unusual costs less than hiding an asset they own.
function isLowHolderCount(holders) {
return holders != null && holders < LOW_HOLDER_THRESHOLD;
}
module.exports = {
LOW_HOLDER_THRESHOLD,
parseHoldersCount,
isLowHolderCount,
};

View File

@@ -9,6 +9,7 @@
const { formatEther, formatUnits } = require("ethers"); const { formatEther, formatUnits } = require("ethers");
const { log, debugFetch } = require("./log"); const { log, debugFetch } = require("./log");
const { KNOWN_SYMBOLS, TOKEN_BY_ADDRESS } = require("./tokenList"); const { KNOWN_SYMBOLS, TOKEN_BY_ADDRESS } = require("./tokenList");
const { parseHoldersCount, isLowHolderCount } = require("./holders");
// Ethereum addresses are case-insensitive: EIP-55 mixed case is a checksum // Ethereum addresses are case-insensitive: EIP-55 mixed case is a checksum
// over the address, not part of its identity. Every address comparison in // over the address, not part of its identity. Every address comparison in
@@ -116,7 +117,10 @@ function parseTokenTransfer(tt, addrLower) {
contractAddress: normalizeAddress( contractAddress: normalizeAddress(
tt.token?.address_hash || tt.token?.address || "", tt.token?.address_hash || tt.token?.address || "",
), ),
holders: parseInt(tt.token?.holders_count || "0", 10), // 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.
holders: parseHoldersCount(tt.token?.holders_count),
}; };
} }
@@ -292,12 +296,13 @@ function filterTransactions(txs, filters = {}) {
continue; continue;
} }
// Filter low-holder tokens (<1000) if setting is on // Filter low-holder tokens (<1000) if setting is on. A token whose
// holder count the explorer did not report is kept: only a reported
// count below the threshold is "low".
if ( if (
filters.hideLowHolderTokens && filters.hideLowHolderTokens &&
tx.contractAddress && tx.contractAddress &&
tx.holders !== null && isLowHolderCount(tx.holders)
tx.holders < 1000
) { ) {
continue; continue;
} }

166
tests/holders.test.js Normal file
View File

@@ -0,0 +1,166 @@
// Tests for src/shared/holders.js and the balance-list spam gate that reads
// it (issue #230).
//
// The rule these pin down: an explorer that reports no holders_count has told
// us nothing, and "nothing" must not be recorded as "zero holders". Zero is
// the strongest spam signal the wallet has, so handing it out for free turns
// a missing field into a hidden asset.
jest.mock("../src/shared/log", () => ({
log: {
debugf: () => {},
infof: () => {},
warnf: () => {},
errorf: () => {},
},
debugFetch: jest.fn(),
setRuntimeDebug: () => {},
isDebug: () => false,
}));
global.fetch = jest.fn(() => {
throw new Error("tests must not perform network requests");
});
global.chrome = { storage: { local: {} } };
const {
LOW_HOLDER_THRESHOLD,
parseHoldersCount,
isLowHolderCount,
} = require("../src/shared/holders");
const { fetchTokenBalances } = require("../src/shared/balances");
const { debugFetch } = require("../src/shared/log");
const BLOCKSCOUT = "https://eth.blockscout.com/api/v2";
const HOLDER = "0x66133e8ea0f5d1d612d2502a968757d1048c214a";
const USDC_CONTRACT = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48";
const NOVEL_TOKEN = "0x1111111111111111111111111111111111111111";
describe("parseHoldersCount", () => {
test("a reported count parses to that number", () => {
expect(parseHoldersCount("3500000")).toBe(3500000);
expect(parseHoldersCount(3500000)).toBe(3500000);
});
test('a reported "0" parses to 0, which is not null', () => {
expect(parseHoldersCount("0")).toBe(0);
expect(parseHoldersCount(0)).toBe(0);
});
test("an omitted, null or empty count is unknown", () => {
expect(parseHoldersCount(undefined)).toBeNull();
expect(parseHoldersCount(null)).toBeNull();
expect(parseHoldersCount("")).toBeNull();
});
test("an unparseable count is unknown rather than zero", () => {
expect(parseHoldersCount("many")).toBeNull();
expect(parseHoldersCount(NaN)).toBeNull();
});
});
describe("isLowHolderCount", () => {
test("the threshold is the documented 1,000 holders", () => {
expect(LOW_HOLDER_THRESHOLD).toBe(1000);
});
test("a reported count below the threshold is low", () => {
expect(isLowHolderCount(0)).toBe(true);
expect(isLowHolderCount(999)).toBe(true);
});
test("a reported count at or above the threshold is not low", () => {
expect(isLowHolderCount(1000)).toBe(false);
expect(isLowHolderCount(1001)).toBe(false);
});
test("an unknown count is not low", () => {
expect(isLowHolderCount(null)).toBe(false);
expect(isLowHolderCount(undefined)).toBe(false);
});
});
// fetchTokenBalances applies its own spam gate, which is not the low-holder
// display filter: it has no setting behind it and decides what the balance
// list contains at all. It stays strict on an unknown count — see the
// comment at the gate — but must stop recording that unknown as zero.
describe("the balance-list spam gate", () => {
function respondWith(items) {
debugFetch.mockImplementation(async () => ({
ok: true,
status: 200,
statusText: "OK",
json: async () => items,
}));
}
function item(overrides = {}) {
const { token, ...rest } = overrides;
return {
value: "12500000",
...rest,
token: {
type: "ERC-20",
address_hash: NOVEL_TOKEN,
symbol: "SPAMTKN",
name: "Spam Token",
decimals: "6",
holders_count: "50000",
...token,
},
};
}
beforeEach(() => {
debugFetch.mockReset();
});
test("a token with plenty of reported holders is listed", async () => {
respondWith([item()]);
const balances = await fetchTokenBalances(HOLDER, BLOCKSCOUT, []);
expect(balances).toHaveLength(1);
expect(balances[0].holders).toBe(50000);
});
test("a token reporting zero holders is still excluded", async () => {
respondWith([item({ token: { holders_count: "0" } })]);
expect(await fetchTokenBalances(HOLDER, BLOCKSCOUT, [])).toEqual([]);
});
test("an unknown holder count does not admit an unvouched token", async () => {
respondWith([item({ token: { holders_count: null } })]);
expect(await fetchTokenBalances(HOLDER, BLOCKSCOUT, [])).toEqual([]);
});
// The path that reaches the send selector and the history filter: a token
// the user vouched for by tracking it is listed whatever the explorer
// says, and it must carry the unknown count through as null, not as the
// zero that would then hide it downstream.
test("a tracked token with an unknown count is listed with holders null", async () => {
respondWith([item({ token: { holders_count: undefined } })]);
const balances = await fetchTokenBalances(HOLDER, BLOCKSCOUT, [
{ address: NOVEL_TOKEN.toUpperCase() },
]);
expect(balances).toHaveLength(1);
expect(balances[0].holders).toBeNull();
});
test("a known-list token with an unknown count is listed with holders null", async () => {
respondWith([
item({
token: {
address_hash: USDC_CONTRACT,
symbol: "USDC",
holders_count: null,
},
}),
]);
const balances = await fetchTokenBalances(HOLDER, BLOCKSCOUT, []);
expect(balances).toHaveLength(1);
expect(balances[0].holders).toBeNull();
});
test("no test in this file performed a network request", () => {
expect(global.fetch).not.toHaveBeenCalled();
});
});

View File

@@ -0,0 +1,123 @@
// Tests for the token filtering in the Send view's token selector
// (src/popup/views/send.js).
//
// The selector decides which of the user's tokens can be spent at all, so
// over-filtering here is worse than in the history list: the asset is not
// merely hidden, it becomes unspendable through the UI. Issue #230: an
// explorer that omits holders_count was read as "zero holders" and the token
// disappeared from this list.
//
// renderSendTokenSelect only ever touches getElementById, createElement,
// innerHTML, value, textContent and appendChild, so a small stub document is
// enough to drive it; the real DOM behaviour of the view is covered by
// tests/e2e/run.js.
globalThis.chrome = {
storage: { local: { get: async () => ({}), set: async () => {} } },
};
const { state } = require("../src/shared/state");
const { renderSendTokenSelect } = require("../src/popup/views/send");
const USDC_CONTRACT = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
const NOVEL_TOKEN = "0x1111111111111111111111111111111111111111";
let select;
function installStubDocument() {
select = { innerHTML: "", children: [] };
select.appendChild = (child) => select.children.push(child);
globalThis.document = {
getElementById: (id) => (id === "send-token" ? select : null),
createElement: () => ({ value: "", textContent: "" }),
};
}
// The symbols offered for sending, excluding the hardcoded ETH option that
// renderSendTokenSelect writes straight into innerHTML.
function offeredTokens() {
return select.children.map((opt) => opt.value.toLowerCase());
}
function tokenBalance(overrides) {
return {
address: NOVEL_TOKEN,
symbol: "SPAMTKN",
decimals: 18,
balance: "12.5",
holders: 50000,
...overrides,
};
}
function render(tokenBalances) {
installStubDocument();
renderSendTokenSelect({ address: "0x" + "a".repeat(40), tokenBalances });
}
beforeEach(() => {
state.fraudContracts = [];
state.hideLowHolderTokens = true;
});
describe("the low-holder rule in the send token selector", () => {
test("ETH is always offered", () => {
render([]);
expect(select.innerHTML).toBe('<option value="ETH">ETH</option>');
expect(offeredTokens()).toEqual([]);
});
test("a token with plenty of holders is offered", () => {
render([tokenBalance()]);
expect(offeredTokens()).toEqual([NOVEL_TOKEN]);
});
test("a token reporting zero holders is withheld", () => {
render([tokenBalance({ holders: 0 })]);
expect(offeredTokens()).toEqual([]);
});
test("boundary: 999 holders is withheld, 1000 is offered", () => {
render([tokenBalance({ holders: 999 })]);
expect(offeredTokens()).toEqual([]);
render([tokenBalance({ holders: 1000 })]);
expect(offeredTokens()).toEqual([NOVEL_TOKEN]);
});
// Issue #230: an unknown holder count must not read as zero. A token the
// user demonstrably holds — it has a balance — cannot be made unspendable
// by a field the block explorer failed to report.
test("a token whose holder count is unknown is still offered", () => {
render([tokenBalance({ holders: null })]);
expect(offeredTokens()).toEqual([NOVEL_TOKEN]);
});
test("a token balance carrying no holders field at all is offered", () => {
const t = tokenBalance();
delete t.holders;
render([t]);
expect(offeredTokens()).toEqual([NOVEL_TOKEN]);
});
test("the rule is bypassed entirely when the setting is off", () => {
state.hideLowHolderTokens = false;
render([tokenBalance({ holders: 0 })]);
expect(offeredTokens()).toEqual([NOVEL_TOKEN]);
});
});
describe("the other send-selector rules are unaffected", () => {
test("a token spoofing a known symbol from a wrong address is withheld", () => {
render([
tokenBalance({ symbol: "USDC", holders: null }),
tokenBalance({ address: USDC_CONTRACT, symbol: "USDC" }),
]);
expect(offeredTokens()).toEqual([USDC_CONTRACT.toLowerCase()]);
});
test("a blocklisted fraud contract is withheld even with an unknown count", () => {
state.fraudContracts = [NOVEL_TOKEN.toUpperCase()];
render([tokenBalance({ holders: null })]);
expect(offeredTokens()).toEqual([]);
});
});

View File

@@ -1473,6 +1473,65 @@ describe("fetchRecentTransactions merge and dedup", () => {
expect(result.newFraudContracts).toEqual([FAKE_ETH_CONTRACT]); expect(result.newFraudContracts).toEqual([FAKE_ETH_CONTRACT]);
}); });
// Regression guards (#230): the explorer's holders_count is optional. A
// missing field means the count is unknown; it does not mean the token
// has no holders. Recording the two as the same number both hides a
// legitimate token and makes the `holders !== null` guard in
// filterTransactions unreachable for token transfers.
describe("an unreported holders_count is unknown, not zero", () => {
function spamTransferWithToken(token) {
return [
{
transaction_hash: "0x" + "9".repeat(64),
block_number: 21000070,
timestamp: TS,
from: { hash: ORDINARY_PEER },
to: { hash: VICTIM },
total: { value: "1500500000", decimals: "6" },
token: token,
},
];
}
const OMITTED = {
symbol: NOVEL_SPAM_SYMBOL,
address_hash: NOVEL_SPAM_CONTRACT,
};
const NULLED = { ...OMITTED, holders_count: null };
const ZERO = { ...OMITTED, holders_count: "0" };
test("an omitted holders_count parses to null", async () => {
respondWith([], spamTransferWithToken(OMITTED));
const txs = await fetchRecentTransactions(VICTIM, BLOCKSCOUT);
expect(txs[0].holders).toBeNull();
});
test("a null holders_count parses to null", async () => {
respondWith([], spamTransferWithToken(NULLED));
const txs = await fetchRecentTransactions(VICTIM, BLOCKSCOUT);
expect(txs[0].holders).toBeNull();
});
test("the transfer survives the low-holder filter", async () => {
respondWith([], spamTransferWithToken(OMITTED));
const txs = await fetchRecentTransactions(VICTIM, BLOCKSCOUT);
expect(filterTransactions(txs, filters()).transactions).toEqual(
txs,
);
});
// The regression this fix could cause: a token that genuinely
// reports zero holders must keep being filtered. Unlike the fake
// "ETH" fixture above, this symbol is not in the token list, so the
// holder count is the only rule that can catch it.
test('a reported holders_count of "0" still parses to 0 and is filtered', async () => {
respondWith([], spamTransferWithToken(ZERO));
const txs = await fetchRecentTransactions(VICTIM, BLOCKSCOUT);
expect(txs[0].holders).toBe(0);
expect(filterTransactions(txs, filters()).transactions).toEqual([]);
});
});
test("failed responses yield an empty list rather than throwing", async () => { test("failed responses yield an empty list rather than throwing", async () => {
debugFetch.mockImplementation(async () => ({ debugFetch.mockImplementation(async () => ({
ok: false, ok: false,