sneak/AutistMask
1
0
Fork 0
Code Issues 34 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix: an address holding only unpriced tokens is no longer totalled at $0.00 (closes #261) #298

Merged
clawbot merged 1 commits from issue-261-unknown-value into next 2026-08-17 08:38:11 +02:00
Conversation 2 Commits 1 Files Changed 10 +370 -64
clawbot commented 2026-08-17 08:10:40 +02:00
Collaborator
Copy Link

Closes #261.

Prices are fetched for the top 25 tokens only, so an address can hold real assets this build has no price for. The address total summed the priced holdings and printed that as the total, so an address holding nothing but unpriced ERC-20s was reported as worth $0.00 — wrong in the direction that matters, and on the address-removal confirmation it sat directly under "This address holds a balance."

The decision

getAddressValue() returns { usd, partial } — the value of the priced holdings, and whether an unpriced holding was left out of it. Worth zero and worth an unknown amount stay separate facts, the same distinction #230 drew for an absent holders_count; the rule lives in one module and every screen reads it through the one formatter, as that fix put the holder rule in src/shared/holders.js.

formatAddressTotal() is that single rendering:

state line
nothing knowable (testnet, before the first fetch) no total line, unchanged
everything priced Total: $5,500.00, unchanged
part priced Total: $3,000.00 plus unpriced tokens
nothing priced, something held Total: unpriced tokens only

A partial total is kept and labelled, not suppressed. The figure is the ETH and priced tokens the user does hold; it is correct as far as it goes, so it is named as a floor rather than thrown away. Suppressing the line — the alternative I rejected — would lose that figure on every address holding one unpriced airdrop, which is most of them.

What is never printed is a figure covering no holdings at all. $0.00 there is the sum of an empty set, not a valuation, and it is the sentence-level contradiction the issue reports. Total: unpriced tokens only says exactly what is known. Labelling that case Total: $0.00 plus unpriced tokens was also rejected: it still puts $0.00 under "This address holds a balance." and a skimming user reads the number, not the qualifier.

An address that genuinely holds nothing is unaffected: Total: $0.00, complete, and distinguishable from every case above. The per-token balance lines are untouched — a token with no price shows its quantity and a blank USD column, which is where the "unpriced" claim is visible in detail.

The two new strings are value labels in the established Total: <value> form, not error text, so RULES.md Language & Labeling's full-sentence requirement for error messages does not apply to them; the prose explanation lives in the README section instead.

Call sites

All four consumers, found by grep rather than from the issue's list:

  • src/popup/views/home.js — the summary line under the active address's ETH balance
  • src/popup/views/home.js — the per-address total in the wallet list (previously a bare figure with no Total: label; now labelled like the others, which is what "the same data formatted identically everywhere" asks for)
  • src/popup/views/addressDetail.js — address-usd-total
  • src/popup/views/deleteAddress.js — the balance warning on the removal confirmation

getWalletValueUsd()/getTotalValueUsd() had the same defect and consumed the helper; they are now getWalletValue()/getTotalValue() and carry partial up, so a future consumer cannot lose it. Nothing calls them today. helpers.js and addressToken.js imported the old helper without using it; the dead imports are gone. The rename means a missed call site is a TypeError, not a silently wrong number.

README.md gains a "Partial USD totals" section under Display Consistency, and the three screen descriptions point at it.

Failing first

tests/addressValue.test.js was written before the fix and run against the unfixed helper with make test: 19 failed, 706 passed. The two that carry the bug:

● the wallet list on Home › an address holding only unpriced tokens is not totalled at $0.00
  Expected: "Total: unpriced tokens only"
  Received: "$0.00"

● the balance warning on the address-removal confirmation › an address holding only unpriced tokens is not totalled at $0.00
  Expected: "Total: unpriced tokens only"
  Received: "Total: $0.00"

The fixture is an address with no ETH holding 5000 units of a token with no price, with ETH and USDC priced — so the failure is the real bug and not a missing price feed. The assertions read the total line specifically, because the ETH balance line above it legitimately reads $0.00 for an address with no ETH.

Covered: only-unpriced, genuinely-zero, fully-priced and part-priced, at the helper, at the formatter, and through both call sites that return their markup as a string (walletListHtml(), balanceWarningHtml()). Also the zero-balance unpriced token (not a holding, so the total stays complete), the no-prices-at-all state, and partial propagating through getWalletValue()/getTotalValue(). AddressDetail and the Home summary line render straight into the DOM; both are one call to the shared formatter and are exercised by tests/e2e, which was not run here.

Verification

make check green on the rebased branch: 30 suites, 725 tests, 18 verify-build cases, prettier --check clean. make build also run, emitting dist/chrome/ and dist/firefox/ with DEBUG off.

Closes [#261](https://git.eeqj.de/sneak/AutistMask/issues/261). Prices are fetched for the top 25 tokens only, so an address can hold real assets this build has no price for. The address total summed the priced holdings and printed that as the total, so an address holding nothing but unpriced ERC-20s was reported as worth `$0.00` — wrong in the direction that matters, and on the address-removal confirmation it sat directly under "This address holds a balance." ## The decision `getAddressValue()` returns `{ usd, partial }` — the value of the priced holdings, and whether an unpriced holding was left out of it. Worth zero and worth an unknown amount stay separate facts, the same distinction [#230](https://git.eeqj.de/sneak/AutistMask/issues/230) drew for an absent `holders_count`; the rule lives in one module and every screen reads it through the one formatter, as that fix put the holder rule in `src/shared/holders.js`. `formatAddressTotal()` is that single rendering: | state | line | | --- | --- | | nothing knowable (testnet, before the first fetch) | no total line, unchanged | | everything priced | `Total: $5,500.00`, unchanged | | part priced | `Total: $3,000.00 plus unpriced tokens` | | nothing priced, something held | `Total: unpriced tokens only` | **A partial total is kept and labelled, not suppressed.** The figure is the ETH and priced tokens the user does hold; it is correct as far as it goes, so it is named as a floor rather than thrown away. Suppressing the line — the alternative I rejected — would lose that figure on every address holding one unpriced airdrop, which is most of them. **What is never printed is a figure covering no holdings at all.** `$0.00` there is the sum of an empty set, not a valuation, and it is the sentence-level contradiction the issue reports. `Total: unpriced tokens only` says exactly what is known. Labelling that case `Total: $0.00 plus unpriced tokens` was also rejected: it still puts `$0.00` under "This address holds a balance." and a skimming user reads the number, not the qualifier. An address that genuinely holds nothing is unaffected: `Total: $0.00`, complete, and distinguishable from every case above. The per-token balance lines are untouched — a token with no price shows its quantity and a blank USD column, which is where the "unpriced" claim is visible in detail. The two new strings are value labels in the established `Total: <value>` form, not error text, so `RULES.md` Language & Labeling's full-sentence requirement for error messages does not apply to them; the prose explanation lives in the README section instead. ## Call sites All four consumers, found by grep rather than from the issue's list: - `src/popup/views/home.js` — the summary line under the active address's ETH balance - `src/popup/views/home.js` — the per-address total in the wallet list (previously a bare figure with no `Total:` label; now labelled like the others, which is what "the same data formatted identically everywhere" asks for) - `src/popup/views/addressDetail.js` — `address-usd-total` - `src/popup/views/deleteAddress.js` — the balance warning on the removal confirmation `getWalletValueUsd()`/`getTotalValueUsd()` had the same defect and consumed the helper; they are now `getWalletValue()`/`getTotalValue()` and carry `partial` up, so a future consumer cannot lose it. Nothing calls them today. `helpers.js` and `addressToken.js` imported the old helper without using it; the dead imports are gone. The rename means a missed call site is a `TypeError`, not a silently wrong number. `README.md` gains a "Partial USD totals" section under Display Consistency, and the three screen descriptions point at it. ## Failing first `tests/addressValue.test.js` was written before the fix and run against the unfixed helper with `make test`: **19 failed, 706 passed**. The two that carry the bug: ● the wallet list on Home › an address holding only unpriced tokens is not totalled at $0.00 Expected: "Total: unpriced tokens only" Received: "$0.00" ● the balance warning on the address-removal confirmation › an address holding only unpriced tokens is not totalled at $0.00 Expected: "Total: unpriced tokens only" Received: "Total: $0.00" The fixture is an address with no ETH holding 5000 units of a token with no price, with `ETH` and `USDC` priced — so the failure is the real bug and not a missing price feed. The assertions read the total line specifically, because the ETH balance line above it legitimately reads `$0.00` for an address with no ETH. Covered: only-unpriced, genuinely-zero, fully-priced and part-priced, at the helper, at the formatter, and through both call sites that return their markup as a string (`walletListHtml()`, `balanceWarningHtml()`). Also the zero-balance unpriced token (not a holding, so the total stays complete), the no-prices-at-all state, and `partial` propagating through `getWalletValue()`/`getTotalValue()`. AddressDetail and the Home summary line render straight into the DOM; both are one call to the shared formatter and are exercised by `tests/e2e`, which was not run here. ## Verification `make check` green on the rebased branch: 30 suites, 725 tests, 18 verify-build cases, `prettier --check` clean. `make build` also run, emitting `dist/chrome/` and `dist/firefox/` with `DEBUG` off.
clawbot added the needs-review label 2026-08-17 08:10:40 +02:00
clawbot added 1 commit 2026-08-17 08:10:40 +02:00
fix: an address holding only unpriced tokens is no longer totalled at $0.00 (closes #261)
Some checks failed
check / check (push) Has been cancelled
Details
7d97ea4b53 
Prices are fetched for the top 25 tokens only, so an address can hold real
assets this build has no price for. The address total summed the priced
holdings and printed the result as the total, so an address holding nothing
but unpriced ERC-20s was reported as worth $0.00 — wrong in the direction
that matters, and on the address-removal confirmation it sat directly under
"This address holds a balance."

getAddressValue() returns { usd, partial }: the value of the priced holdings,
and whether an unpriced holding was left out of it. Worth zero and worth an
unknown amount stay separate facts, as an absent holders_count stays separate
from a count of zero. formatAddressTotal() is the one rendering of that pair,
so no screen can word it differently:

  - nothing knowable (testnet, before the first fetch): no total line
  - everything priced:  "Total: $5,500.00"
  - part priced:        "Total: $3,000.00 plus unpriced tokens"
  - nothing priced:     "Total: unpriced tokens only"

A partial total is kept rather than suppressed: the figure is the ETH and
priced tokens the user does hold and is correct as far as it goes, so it is
named as a floor instead of being thrown away. What is never printed is a
figure covering no holdings at all.

All four call sites read it — the Home summary line, the Home wallet list,
AddressDetail and the removal confirmation — and getWalletValue() and
getTotalValue() carry partial up so a future consumer cannot lose it.
The per-token balance lines are unchanged: a token with no price shows its
quantity and a blank USD column.

tests/addressValue.test.js covers the only-unpriced, genuinely-zero and
fully-priced cases at the helper, at its formatter, and through both call
sites that return their markup as a string. Written first and watched fail
on the unfixed helper: the Home wallet list gave "$0.00" and the removal
confirmation "Total: $0.00" for an address holding 5000 unpriced tokens.
clawbot self-assigned this 2026-08-17 08:10:53 +02:00
clawbot force-pushed issue-261-unknown-value from 7d97ea4b53 to 9c834f440f 2026-08-17 08:11:21 +02:00 Compare
clawbot commented 2026-08-17 08:16:04 +02:00
Author
Collaborator
Copy Link

FAIL — one finding.

Commit authorship (9c834f4) — git log -1 --format='%an &lt;%ae&gt; | %cn &lt;%ce&gt;' returns sneak &lt;sneak@sneak.berlin&gt; | sneak &lt;sneak@sneak.berlin&gt; on both author and committer. Bot-produced commits must be authored as clawbot (#186); this one misattributes the work to the repo owner and corrupts git blame and git shortlog. Acceptable: recommit (amend/rebase) with clawbot as author and committer, leaving the tree identical.

Everything else verified and passing: DoD boxes all satisfied; all four call sites converted with no occurrence of the old names left anywhere in the tree; the removed helpers.js and addressToken.js imports were genuinely unused at base; failing-first reproduced locally (neutering partial in getAddressValue() fails 9 tests, including both quoted bug assertions, which read the total line specifically); make check green in a fresh clone (30 suites / 725 tests, prettier clean); CI green on 9c834f4; single commit, base next, title carries (closes #261), TODO.md in the same commit with no prior entry dropped; no attribution trailers or vendor references.

Two disclosures. RULES.md Language & Labeling's full-sentence rule covers error messages only (line 120), so the author's reading is correct and the two new value labels are in scope of the existing Total: &lt;value&gt; form. The AddressDetail / Home-summary DOM paths are waived as unit-untestable: both are a single call to formatAddressTotal(), which is directly covered, and tests/e2e was not run here given the recorded flake in #287 / #290.

FAIL — one finding. **Commit authorship (`9c834f4`)** — `git log -1 --format='%an &lt;%ae&gt; | %cn &lt;%ce&gt;'` returns `sneak &lt;sneak@sneak.berlin&gt; | sneak &lt;sneak@sneak.berlin&gt;` on both author and committer. Bot-produced commits must be authored as `clawbot` ([#186](https://git.eeqj.de/sneak/AutistMask/issues/186)); this one misattributes the work to the repo owner and corrupts `git blame` and `git shortlog`. Acceptable: recommit (amend/rebase) with `clawbot` as author and committer, leaving the tree identical. Everything else verified and passing: DoD boxes all satisfied; all four call sites converted with no occurrence of the old names left anywhere in the tree; the removed `helpers.js` and `addressToken.js` imports were genuinely unused at base; failing-first reproduced locally (neutering `partial` in `getAddressValue()` fails 9 tests, including both quoted bug assertions, which read the total line specifically); `make check` green in a fresh clone (30 suites / 725 tests, prettier clean); CI green on `9c834f4`; single commit, base `next`, title carries ` (closes #261)`, `TODO.md` in the same commit with no prior entry dropped; no attribution trailers or vendor references. Two disclosures. `RULES.md` Language &amp; Labeling's full-sentence rule covers error messages only (line 120), so the author's reading is correct and the two new value labels are in scope of the existing `Total: &lt;value&gt;` form. The AddressDetail / Home-summary DOM paths are waived as unit-untestable: both are a single call to `formatAddressTotal()`, which is directly covered, and `tests/e2e` was not run here given the recorded flake in [#287](https://git.eeqj.de/sneak/AutistMask/issues/287) / [#290](https://git.eeqj.de/sneak/AutistMask/issues/290).
clawbot added needs-rework and removed needs-review labels 2026-08-17 08:16:07 +02:00
clawbot commented 2026-08-17 08:38:05 +02:00
Author
Collaborator
Copy Link

PASS. The sole finding was commit authorship; the owner has ruled that sneak identity is correct (#186), so it is withdrawn. No other finding stood. Squash-merging.

PASS. The sole finding was commit authorship; the owner has ruled that `sneak` identity is correct (https://git.eeqj.de/sneak/AutistMask/issues/186), so it is withdrawn. No other finding stood. Squash-merging.
clawbot merged commit e07efb710a into next 2026-08-17 08:38:11 +02:00
clawbot deleted branch issue-261-unknown-value 2026-08-17 08:38:11 +02:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label needs-rework
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees clawbot
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#298
Delete Branch "issue-261-unknown-value"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?