diff --git a/README.md b/README.md index f88d33f..03edbec 100644 --- a/README.md +++ b/README.md @@ -538,6 +538,27 @@ Both are click-copyable. Truncating to 4 decimals in summary views is acceptable for scannability, but the detail view must never discard precision — it is the one place the user can always use to verify exact details. +#### Partial USD totals + +Prices are fetched for the top 25 tokens only, so an address can hold assets the +extension has no price for. Worth zero and worth an unknown amount are different +facts and are never collapsed into one number. `getAddressValue()` in +`src/shared/prices.js` returns `{ usd, partial }` — the value of the priced +holdings, and whether an unpriced holding was left out of it — and every screen +renders it through `formatAddressTotal()`, so the wording cannot drift: + +- Nothing knowable (testnet, or before the first price fetch): no total line. +- Everything priced: `Total: $5,500.00`. +- Part priced: `Total: $3,000.00 plus unpriced tokens` — the figure is real as + far as it goes and is kept, named as a floor rather than the total. +- Nothing priced but something held: `Total: unpriced tokens only`. No figure, + because the only figure available would be the `$0.00` sum of an empty set, + and on the address-removal confirmation that sits directly under "This address + holds a balance." + +The per-token balance lines are unaffected: each shows its quantity, and a USD +column that is blank for a token with no price. + #### Language & Labeling All user-facing text avoids unnecessary jargon wherever possible: @@ -673,7 +694,9 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign. - **When**: At least one wallet exists. This is the root screen. - **Elements**: - Active address ETH balance (large) + USD value in parentheses - - "Total:" USD value across ETH and every token shown for the active address + - "Total:" USD value across ETH and every token shown for the active + address, written by `formatAddressTotal()` — see + [Partial USD totals](#partial-usd-totals) - Active address (color dot, full address, etherscan link, tap to copy) - Send / Receive quick-action buttons, both acting on the active address - ETH/USD price display @@ -735,7 +758,7 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign. - Title: "Wallet Name — Address N" - ENS name (if resolved, bold above the address) - Full address (color dot, etherscan link, tap to copy) - - USD total for address + - USD total for address (see [Partial USD totals](#partial-usd-totals)) - Balance list: ETH + the ERC-20 tokens shown for this address (4 decimal places, USD inline). Each balance row is clickable → **AddressToken** - Send / Receive / + Token buttons and a "···" menu button @@ -1101,11 +1124,13 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign. xprv wallet has no recovery phrase to re-import. - A warning when the address holds anything, ETH or any tracked ERC-20, followed by the holdings themselves via `balanceLinesForAddress()` and the - USD total via `getAddressValueUsd()`. The sentence names no figure of its - own: the lines round to four decimals, so a sentence built from a rounded - number would report `0.0000 ETH` for an address holding real money. The - predicate is `addressHoldsFunds()` in `src/popup/views/helpers.js`, - unrounded and token-aware. A balance is a warning, never a refusal. + USD total via `formatAddressTotal()` (see + [Partial USD totals](#partial-usd-totals)). The sentence names no figure + of its own: the lines round to four decimals, so a sentence built from a + rounded number would report `0.0000 ETH` for an address holding real + money. The predicate is `addressHoldsFunds()` in + `src/popup/views/helpers.js`, unrounded and token-aware. A balance is a + warning, never a refusal. - The rule that a wallet always keeps at least one address, and that removing the last one means deleting the wallet from Settings - Error line diff --git a/TODO.md b/TODO.md index ec7d000..2cb7f66 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,22 @@ undefined identifiers, which is how # Completed Steps +- 2026-08-17: An address total no longer reports `$0.00` for holdings it cannot + price. Prices exist for the top 25 tokens only, so the priced-only sum was + printed as the total and an address holding nothing but unpriced ERC-20s was + shown as worth nothing — directly under "This address holds a balance." on the + address-removal confirmation. `getAddressValue()` in `src/shared/prices.js` + now returns `{ usd, partial }`, keeping worth-zero and worth-an-unknown-amount + apart the way an absent `holders_count` is kept apart from a count of zero, + and every screen renders it through the one `formatAddressTotal()`: the figure + when it covers everything, the figure marked `plus unpriced tokens` when it + covers part, and `Total: unpriced tokens only` when it would cover nothing. + Home, AddressDetail and the removal confirmation all read it, and + `getWalletValue()`/`getTotalValue()` carry `partial` up. Covered by + `tests/addressValue.test.js` — the only-unpriced, genuinely-zero and + fully-priced cases at the helper and at both call sites that return their + markup — demonstrated failing first + ([#261](https://git.eeqj.de/sneak/AutistMask/issues/261)). - 2026-08-17: `README.md` no longer advertises a defect the wallet does not have. The End-to-End Tests section listed the EIP-1193 code being dropped in the last hop into the page as a standing limit of the dApp coverage; that diff --git a/src/popup/views/addressDetail.js b/src/popup/views/addressDetail.js index 105c352..1f04f8a 100644 --- a/src/popup/views/addressDetail.js +++ b/src/popup/views/addressDetail.js @@ -13,7 +13,7 @@ const { pushCurrentView, } = require("./helpers"); const { state, currentAddress, saveState } = require("../../shared/state"); -const { formatUsd, getAddressValueUsd } = require("../../shared/prices"); +const { formatAddressTotal, getAddressValue } = require("../../shared/prices"); const { fetchRecentTransactions, filterTransactions, @@ -64,7 +64,7 @@ function show() { }); $("address-line").dataset.full = addr.address; attachCopyHandlers($("address-line")); - const usdTotal = formatUsd(getAddressValueUsd(addr)); + const usdTotal = formatAddressTotal(getAddressValue(addr)); $("address-usd-total").innerHTML = usdTotal || " "; const ensEl = $("address-ens"); // ENS is now shown inside renderAddressHtml, hide the separate element diff --git a/src/popup/views/addressToken.js b/src/popup/views/addressToken.js index 7dba24c..09dd0e2 100644 --- a/src/popup/views/addressToken.js +++ b/src/popup/views/addressToken.js @@ -18,11 +18,7 @@ const { } = require("./helpers"); const { state, currentAddress, saveState } = require("../../shared/state"); const { TOKEN_BY_ADDRESS, resolveSymbol } = require("../../shared/tokenList"); -const { - formatUsd, - getPrice, - getAddressValueUsd, -} = require("../../shared/prices"); +const { formatUsd, getPrice } = require("../../shared/prices"); const { fetchRecentTransactions, filterTransactions, diff --git a/src/popup/views/deleteAddress.js b/src/popup/views/deleteAddress.js index ca7af8a..7263b07 100644 --- a/src/popup/views/deleteAddress.js +++ b/src/popup/views/deleteAddress.js @@ -17,7 +17,7 @@ const { addressHoldsFunds, balanceLinesForAddress, } = require("./helpers"); -const { formatUsd, getAddressValueUsd } = require("../../shared/prices"); +const { formatAddressTotal, getAddressValue } = require("../../shared/prices"); const { walletHasRecoveryPhrase } = require("../../shared/wallet"); const { state, saveState } = require("../../shared/state"); const { @@ -84,16 +84,16 @@ function recoveryPathText(wallet) { // own: the rendered lines round to four decimals, so a sentence built from a // rounded number would report "0.0000 ETH" for an address holding real money. // The lines below it carry the amounts, in the same format as Home and -// AddressDetail, followed by the USD total when prices are known (null on -// testnet and before the first price fetch, where the line is left off rather -// than printed as $0.00). +// AddressDetail, followed by the USD total when there is one to give — no +// total line at all on testnet or before the first price fetch, and no figure +// when every holding here is one with no price, since "$0.00" directly under +// "This address holds a balance." is a contradiction. function balanceWarningHtml(addr) { if (!addressHoldsFunds(addr)) return " "; - const usd = getAddressValueUsd(addr); - const total = - usd === null - ? "" - : `
Total: ${formatUsd(usd)}
`; + const line = formatAddressTotal(getAddressValue(addr)); + const total = line + ? `
${line}
` + : ""; return ( `

This address holds a balance. Removing it does not ` + `move or spend anything; the balance stays at the address.

` + diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index d16885d..6b6b891 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -1,11 +1,7 @@ // Shared DOM helpers used by all views. const { isDebug } = require("../../shared/log"); -const { - formatUsd, - getPrice, - getAddressValueUsd, -} = require("../../shared/prices"); +const { formatUsd, getPrice } = require("../../shared/prices"); const { state, saveState, currentNetwork } = require("../../shared/state"); const { markViewRendered } = require("../viewRouter"); diff --git a/src/popup/views/home.js b/src/popup/views/home.js index bdf1e6f..24d085c 100644 --- a/src/popup/views/home.js +++ b/src/popup/views/home.js @@ -28,8 +28,9 @@ const { } = require("../../shared/walletDefects"); const { formatUsd, + formatAddressTotal, getPrice, - getAddressValueUsd, + getAddressValue, } = require("../../shared/prices"); const { fetchRecentTransactions, @@ -71,9 +72,7 @@ function renderTotalValue() { el.textContent = ethStr + ethUsd; if (subEl) { - const totalUsd = getAddressValueUsd(addr); - subEl.innerHTML = - totalUsd !== null ? "Total: " + formatUsd(totalUsd) : " "; + subEl.innerHTML = formatAddressTotal(getAddressValue(addr)) || " "; } } @@ -257,8 +256,8 @@ function walletListHtml() { html += `${addr.ensName ? "" : dot}${addr.address}`; html += `${infoBtn}${removeBtn}`; html += ``; - const addrUsd = formatUsd(getAddressValueUsd(addr)); - html += `
${addrUsd || " "}
`; + const addrTotal = formatAddressTotal(getAddressValue(addr)); + html += `
${addrTotal || " "}
`; html += balanceLinesForAddress( addr, state.trackedTokens, diff --git a/src/shared/prices.js b/src/shared/prices.js index ac45dd1..4cde335 100644 --- a/src/shared/prices.js +++ b/src/shared/prices.js @@ -55,42 +55,77 @@ function formatUsd(amount) { ); } -function getAddressValueUsd(addr) { +// What an address is worth, as { usd, partial }. +// +// Prices are fetched for the top 25 tokens only, so an address can hold real +// assets this code has no price for. Adding up the priced ones and calling the +// result the total states a number the holdings do not support: an address +// holding nothing but unpriced tokens comes out at $0.00, which tells the user +// their address is worth nothing when it may hold a great deal. Worth zero and +// worth an unknown amount are separate facts and get separate fields, the same +// way an absent holders_count is not a count of zero. +// +// usd: the value of the holdings a price is known for, or null when +// nothing is knowable at all — testnet, or before the first fetch. +// partial: the address also holds a token with no price, so usd is a floor +// and not the total. +// +// Render it through formatAddressTotal() rather than reading usd alone. +function getAddressValue(addr) { const { currentNetwork } = require("./state"); - if (currentNetwork().isTestnet) return null; - if (!prices.ETH) return null; - let total = 0; - const ethBal = parseFloat(addr.balance || "0"); - total += ethBal * prices.ETH; + if (currentNetwork().isTestnet) return { usd: null, partial: false }; + if (!prices.ETH) return { usd: null, partial: false }; + let usd = parseFloat(addr.balance || "0") * prices.ETH; + let partial = false; for (const token of addr.tokenBalances || []) { const tokenBal = parseFloat(token.balance || "0"); - if (tokenBal > 0 && prices[token.symbol]) { - total += tokenBal * prices[token.symbol]; + // A balance of zero is not a holding: it can neither add to the total + // nor make it incomplete. + if (!(tokenBal > 0)) continue; + if (prices[token.symbol]) { + usd += tokenBal * prices[token.symbol]; + } else { + partial = true; } } - return total; + return { usd, partial }; } -function getWalletValueUsd(wallet) { - const { currentNetwork } = require("./state"); - if (currentNetwork().isTestnet) return null; - if (!prices.ETH) return null; - let total = 0; - for (const addr of wallet.addresses) { - total += getAddressValueUsd(addr); - } - return total; +// The same pair for a whole wallet, and for every wallet at once. One +// unpriced holding anywhere makes the sum a floor, so partial carries up. +function getWalletValue(wallet) { + return sumValues(wallet.addresses.map(getAddressValue)); } -function getTotalValueUsd(wallets) { - const { currentNetwork } = require("./state"); - if (currentNetwork().isTestnet) return null; - if (!prices.ETH) return null; - let total = 0; - for (const wallet of wallets) { - total += getWalletValueUsd(wallet); +function getTotalValue(wallets) { + return sumValues(wallets.map(getWalletValue)); +} + +function sumValues(values) { + let usd = null; + let partial = false; + for (const value of values) { + if (value.usd === null) continue; + usd = (usd === null ? 0 : usd) + value.usd; + partial = partial || value.partial; } - return total; + return { usd, partial }; +} + +// The one rendering of an address total, so no screen says it differently. +// +// A partial total is shown and named as partial: the figure is the ETH and +// priced tokens the user does hold, which is worth having, and suppressing it +// would throw away a number that is correct as far as it goes. What is never +// shown is a figure covering no holdings at all — the $0.00 sum of an empty +// set beside a list of tokens is the bug this replaces. +function formatAddressTotal(value) { + if (!value || value.usd === null) return ""; + if (!value.partial) return "Total: " + formatUsd(value.usd); + if (value.usd > 0) { + return "Total: " + formatUsd(value.usd) + " plus unpriced tokens"; + } + return "Total: unpriced tokens only"; } module.exports = { @@ -99,7 +134,8 @@ module.exports = { clearPrices, getPrice, formatUsd, - getAddressValueUsd, - getWalletValueUsd, - getTotalValueUsd, + formatAddressTotal, + getAddressValue, + getWalletValue, + getTotalValue, }; diff --git a/tests/addressValue.test.js b/tests/addressValue.test.js new file mode 100644 index 0000000..44de5d4 --- /dev/null +++ b/tests/addressValue.test.js @@ -0,0 +1,238 @@ +// The USD total of an address that holds something this build cannot price +// (issue #261). +// +// Prices exist for the top 25 tokens only, so an address can hold real assets +// with no price attached. Summing what is priced and printing the result as +// the total says "$0.00" for an address holding nothing but unpriced tokens — +// worth-nothing and worth-an-unknown-amount collapsed into one number, in the +// direction that matters. The two are separate facts here, the same way an +// absent holders_count is not a count of zero. +// +// The value and its rendering are asserted directly, and then through the two +// call sites that return their markup as a string: the wallet list on Home and +// the balance warning on the address-removal confirmation. AddressDetail and +// the Home summary line render into the DOM and are covered by tests/e2e. + +// helpers.js pulls in state.js, which reads chrome.storage.local at load. +globalThis.chrome = { + storage: { local: { get: async () => ({}), set: async () => {} } }, +}; + +const { + prices, + clearPrices, + getAddressValue, + getWalletValue, + getTotalValue, + formatAddressTotal, +} = require("../src/shared/prices"); +const { state } = require("../src/shared/state"); +const { walletListHtml } = require("../src/popup/views/home"); +const { balanceWarningHtml } = require("../src/popup/views/deleteAddress"); + +const USDC = "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48"; +const NOVEL = "0x1111111111111111111111111111111111111111"; + +// No ETH, and a token no price is known for. The case the user is told is +// worth $0.00 today. +const UNPRICED_ONLY = { + address: "0x" + "a".repeat(40), + balance: "0", + tokenBalances: [{ address: NOVEL, symbol: "NOVEL", balance: "5000.0" }], +}; + +// Nothing at all: the address really is worth zero. +const EMPTY = { + address: "0x" + "b".repeat(40), + balance: "0", + tokenBalances: [], +}; + +// Every holding priced. +const FULLY_PRICED = { + address: "0x" + "c".repeat(40), + balance: "1.5", + tokenBalances: [{ address: USDC, symbol: "USDC", balance: "2500.0" }], +}; + +// Part priced, part not: 1.5 ETH plus a token with no price. +const PARTLY_PRICED = { + address: "0x" + "d".repeat(40), + balance: "1.5", + tokenBalances: [{ address: NOVEL, symbol: "NOVEL", balance: "5000.0" }], +}; + +beforeEach(() => { + clearPrices(); + prices.ETH = 2000; + prices.USDC = 1; + state.wallets = []; + state.trackedTokens = []; + state.showZeroBalanceTokens = false; + state.activeAddress = null; +}); + +afterEach(() => { + clearPrices(); +}); + +// The total line only, in each of the two markup-returning call sites. The +// ETH balance line above it legitimately reads $0.00 for an address with no +// ETH, so the assertions have to name the line under test. +function walletListTotal(addr) { + state.wallets = [{ name: "Wallet 1", type: "hd", addresses: [addr] }]; + const match = walletListHtml().match(/min-h-\[1rem\]">([^<]*)([^<]*) { + test("an address holding only unpriced tokens has an incomplete value", () => { + expect(getAddressValue(UNPRICED_ONLY)).toEqual({ + usd: 0, + partial: true, + }); + }); + + test("an address holding nothing is complete, and zero", () => { + expect(getAddressValue(EMPTY)).toEqual({ usd: 0, partial: false }); + }); + + test("a fully priced address is complete, and unchanged", () => { + expect(getAddressValue(FULLY_PRICED)).toEqual({ + usd: 5500, + partial: false, + }); + }); + + test("a partly priced address keeps the part it can price", () => { + expect(getAddressValue(PARTLY_PRICED)).toEqual({ + usd: 3000, + partial: true, + }); + }); + + // A token balance of zero is not a holding, so it cannot make the total + // incomplete: an address with a spent-out unpriced token is worth zero. + test("a zero balance in an unpriced token leaves the value complete", () => { + const addr = { + address: "0x1", + balance: "0", + tokenBalances: [{ address: NOVEL, symbol: "NOVEL", balance: "0" }], + }; + expect(getAddressValue(addr)).toEqual({ usd: 0, partial: false }); + }); + + // Before the first price fetch, and on testnet, nothing is knowable: that + // is a third state, and it stays distinct from both of the others. + test("no prices at all means no value, not an incomplete one", () => { + clearPrices(); + expect(getAddressValue(FULLY_PRICED)).toEqual({ + usd: null, + partial: false, + }); + }); + + test("one unpriced holding makes a wallet and the grand total partial", () => { + const wallet = { addresses: [FULLY_PRICED, UNPRICED_ONLY] }; + expect(getWalletValue(wallet)).toEqual({ usd: 5500, partial: true }); + expect(getTotalValue([wallet])).toEqual({ usd: 5500, partial: true }); + }); + + test("a wallet of fully priced addresses stays complete", () => { + const wallet = { addresses: [FULLY_PRICED, EMPTY] }; + expect(getWalletValue(wallet)).toEqual({ usd: 5500, partial: false }); + }); +}); + +describe("how that value is written on screen", () => { + test("a complete total is the figure", () => { + expect(formatAddressTotal(getAddressValue(FULLY_PRICED))).toBe( + "Total: $5,500.00", + ); + }); + + test("an address worth zero says so", () => { + expect(formatAddressTotal(getAddressValue(EMPTY))).toBe("Total: $0.00"); + }); + + // The figure is still worth having — it is the ETH the user does hold — + // but on its own it understates the address, so it is named as partial. + test("a partly priced total is given, and marked as partial", () => { + expect(formatAddressTotal(getAddressValue(PARTLY_PRICED))).toBe( + "Total: $3,000.00 plus unpriced tokens", + ); + }); + + // Nothing priced is held, so there is no figure to give: printing the + // $0.00 sum of an empty set is the bug. + test("a total with nothing priced in it gives no figure", () => { + const line = formatAddressTotal(getAddressValue(UNPRICED_ONLY)); + expect(line).toBe("Total: unpriced tokens only"); + expect(line).not.toContain("$"); + }); + + test("an unknown value is written as nothing at all", () => { + clearPrices(); + expect(formatAddressTotal(getAddressValue(FULLY_PRICED))).toBe(""); + }); +}); + +describe("the wallet list on Home", () => { + test("an address holding only unpriced tokens is not totalled at $0.00", () => { + expect(walletListTotal(UNPRICED_ONLY)).toBe( + "Total: unpriced tokens only", + ); + }); + + test("an address holding nothing is still totalled at $0.00", () => { + expect(walletListTotal(EMPTY)).toBe("Total: $0.00"); + }); + + test("a fully priced address shows its total", () => { + expect(walletListTotal(FULLY_PRICED)).toBe("Total: $5,500.00"); + }); + + test("a partly priced address shows the priced part, marked partial", () => { + expect(walletListTotal(PARTLY_PRICED)).toBe( + "Total: $3,000.00 plus unpriced tokens", + ); + }); + + test("an address whose value is unknown keeps its blank line", () => { + clearPrices(); + expect(walletListTotal(FULLY_PRICED)).toBe(" "); + }); +}); + +describe("the balance warning on the address-removal confirmation", () => { + // "This address holds a balance." followed by "Total: $0.00" is a flat + // contradiction, on the one screen whose job is to warn. + test("an address holding only unpriced tokens is not totalled at $0.00", () => { + expect(balanceWarningHtml(UNPRICED_ONLY)).toContain( + "This address holds a balance.", + ); + expect(removalWarningTotal(UNPRICED_ONLY)).toBe( + "Total: unpriced tokens only", + ); + }); + + test("a fully priced address still shows its total", () => { + expect(removalWarningTotal(FULLY_PRICED)).toBe("Total: $5,500.00"); + }); + + test("a partly priced address shows the priced part, marked partial", () => { + expect(removalWarningTotal(PARTLY_PRICED)).toBe( + "Total: $3,000.00 plus unpriced tokens", + ); + }); + + test("no total line is written when the value is unknown", () => { + clearPrices(); + expect(removalWarningTotal(FULLY_PRICED)).toBe(null); + }); +}); diff --git a/tests/deleteAddress.test.js b/tests/deleteAddress.test.js index f8d2682..1d89ce5 100644 --- a/tests/deleteAddress.test.js +++ b/tests/deleteAddress.test.js @@ -150,7 +150,7 @@ describe("the balance warning on the removal confirmation", () => { expect(balanceWarningHtml(ETH_ONLY)).toContain("Total: $3,000.00"); }); - // getAddressValueUsd() returns null on testnet and before the first + // getAddressValue() reports no value on testnet and before the first // price fetch. A "Total: $0.00" there would be a lie about the holdings. test("no USD total is shown when prices are not known", () => { expect(balanceWarningHtml(TOKEN_ONLY)).not.toContain("Total:");