fix: an omitted holders_count is coerced to 0, so a legitimate token gets filtered as spam #230

New Issue

clawbot · 2026-08-11T15:10:47+02:00

clawbot commented

2026-08-11 15:10:47 +02:00

src/shared/transactions.js:112 reads holders_count || "0". When the block explorer omits or nulls that field — a fetch hiccup, a newly indexed token, an API change — the token is recorded as having zero holders rather than an unknown count, and the low-holder filter then hides it as spam.

Two consequences, and the second is the more insidious:

A perfectly legitimate token disappears from the user's transaction history because of a transient upstream omission. Same over-filtering class as #179, one layer earlier.
It makes the existing tx.holders !== null guard dead code for token transfers. That guard exists precisely to avoid filtering on an unknown holder count, and the || "0" coercion upstream guarantees it can never fire.

The same pattern appears at src/popup/views/send.js:135 ((t.holders || 0) < 1000), where an unknown count likewise reads as zero and the token is withheld from the send selector — so the user cannot spend a token they hold.

Found while fixing #179 and deliberately left out of its scope, since it may have been intentional.

Implementation requirements

parseTokenTransfer must emit holders: null when the explorer omits or nulls holders_count, distinguishing "unknown" from "zero".
Decide deliberately how an unknown count should be treated by the filter and say why in the PR body. Defaulting to SHOWING the token is the safer direction — hiding a real asset is worse than showing a spam one, and the user can still see the row is unusual. The opposite choice needs an argument.
Apply the same distinction at send.js:135; a token the user holds must not vanish from the send selector because of an upstream omission.
Check for other || 0 / || "0" coercions on values where absent and zero mean different things, and cover them or rule them out in the PR body.

Definition of done

An omitted or null holders_count yields holders: null, not 0.
The holders !== null guard demonstrably fires for such a transfer — with a test that fails against the current code.
A token with an unknown holder count behaves as decided and documented in both the history filter and the send selector.
A genuine zero-holder token is still filtered.
TODO.md updated in the same commit.
make check passes.

`src/shared/transactions.js:112` reads `holders_count || "0"`. When the block explorer omits or nulls that field — a fetch hiccup, a newly indexed token, an API change — the token is recorded as having **zero holders** rather than an unknown count, and the low-holder filter then hides it as spam. Two consequences, and the second is the more insidious: 1. A perfectly legitimate token disappears from the user's transaction history because of a transient upstream omission. Same over-filtering class as https://git.eeqj.de/sneak/AutistMask/issues/179, one layer earlier. 2. It makes the existing `tx.holders !== null` guard **dead code for token transfers**. That guard exists precisely to avoid filtering on an unknown holder count, and the `|| "0"` coercion upstream guarantees it can never fire. The same pattern appears at `src/popup/views/send.js:135` (`(t.holders || 0) < 1000`), where an unknown count likewise reads as zero and the token is withheld from the send selector — so the user cannot spend a token they hold. Found while fixing https://git.eeqj.de/sneak/AutistMask/issues/179 and deliberately left out of its scope, since it may have been intentional. ## Implementation requirements - `parseTokenTransfer` must emit `holders: null` when the explorer omits or nulls `holders_count`, distinguishing "unknown" from "zero". - Decide deliberately how an unknown count should be treated by the filter and say why in the PR body. Defaulting to SHOWING the token is the safer direction — hiding a real asset is worse than showing a spam one, and the user can still see the row is unusual. The opposite choice needs an argument. - Apply the same distinction at `send.js:135`; a token the user holds must not vanish from the send selector because of an upstream omission. - Check for other `|| 0` / `|| "0"` coercions on values where absent and zero mean different things, and cover them or rule them out in the PR body. ## Definition of done - [ ] An omitted or null `holders_count` yields `holders: null`, not `0`. - [ ] The `holders !== null` guard demonstrably fires for such a transfer — with a test that fails against the current code. - [ ] A token with an unknown holder count behaves as decided and documented in both the history filter and the send selector. - [ ] A genuine zero-holder token is still filtered. - [ ] `TODO.md` updated in the same commit. - [ ] `make check` passes.

clawbot added this to the 1.0.0 milestone 2026-08-11 15:10:47 +02:00

clawbot referenced this issue from a commit

2026-08-12 10:24:20 +02:00

fix: treat an unreported holders_count as unknown, not as zero holders (closes #230)

clawbot referenced a pull request that will close this issue

2026-08-12 10:24:57 +02:00

fix: treat an unreported holders_count as unknown, not as zero holders (closes #230) #244

clawbot commented

2026-08-12 10:25:07 +02:00

Implemented in #244.

The null-versus-zero rule and the 1,000-holder threshold now live in one place, src/shared/holders.js, since the rule was open-coded at three call sites and wrong at all three. parseTokenTransfer emits holders: null for an omitted, null or unparseable count; the history filter and renderSendTokenSelect both judge only a reported count, which makes the holders !== null guard live.

Decision on an unknown count: shown in the two user-facing low-holder filters (both sit behind the setting, and an unspendable token is worse than a visible spam row). Still excluded in the balance-list spam gate in fetchTokenBalances — that gate has no off switch and decides the whole balance list, and missing metadata correlates with newly-indexed tokens, i.e. spam airdrops. It now records the unknown as null rather than 0, so a token that reaches the list by being known or tracked is no longer hidden downstream by a zero it never reported. Rationale in full in the PR body.

Verified: the three history tests and two send-selector tests were written first and watched fail on unmodified next; the reported-zero cases pass before and after, so the anti-spam path is pinned in both directions. make check green on the rebased branch — 16 suites, 388 tests, prettier --check clean.

Implemented in [#244](https://git.eeqj.de/sneak/AutistMask/pulls/244). The null-versus-zero rule and the 1,000-holder threshold now live in one place, `src/shared/holders.js`, since the rule was open-coded at three call sites and wrong at all three. `parseTokenTransfer` emits `holders: null` for an omitted, null or unparseable count; the history filter and `renderSendTokenSelect` both judge only a reported count, which makes the `holders !== null` guard live. Decision on an unknown count: **shown** in the two user-facing low-holder filters (both sit behind the setting, and an unspendable token is worse than a visible spam row). **Still excluded** in the balance-list spam gate in `fetchTokenBalances` — that gate has no off switch and decides the whole balance list, and missing metadata correlates with newly-indexed tokens, i.e. spam airdrops. It now records the unknown as `null` rather than `0`, so a token that reaches the list by being known or tracked is no longer hidden downstream by a zero it never reported. Rationale in full in the PR body. Verified: the three history tests and two send-selector tests were written first and watched fail on unmodified `next`; the reported-zero cases pass before and after, so the anti-spam path is pinned in both directions. `make check` green on the rebased branch — 16 suites, 388 tests, `prettier --check` clean.

clawbot referenced this issue

2026-08-12 10:34:01 +02:00

fix: treat an unreported holders_count as unknown, not as zero holders (closes #230) #244

clawbot closed this issue

2026-08-12 10:34:46 +02:00

clawbot referenced this issue from a commit

2026-08-12 10:34:47 +02:00

fix: distinguish an unknown holder count from zero so a legitimate token is not filtered (closes #230)

clawbot referenced this issue

2026-08-12 10:35:02 +02:00

harden: parseHoldersCount partial-parses a malformed count into a reported low count, hiding the token #251

clawbot referenced this issue