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

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

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,

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
-            ? ""
-            : `<div class="text-xs text-muted mt-1">Total: ${formatUsd(usd)}</div>`;
+    const line = formatAddressTotal(getAddressValue(addr));
+    const total = line
+        ? `<div class="text-xs text-muted mt-1">${line}</div>`
+        : "";
     return (
         `<p class="mb-1">This address holds a balance. Removing it does not ` +
         `move or spend anything; the balance stays at the address.</p>` +

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");
 

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 += `<span class="flex items-center break-all">${addr.ensName ? "" : dot}${addr.address}</span>`;
             html += `<span class="flex-shrink-0 ml-1">${infoBtn}${removeBtn}</span>`;
             html += `</div>`;
-            const addrUsd = formatUsd(getAddressValueUsd(addr));
-            html += `<div class="text-xs text-muted text-right min-h-[1rem]">${addrUsd || "&nbsp;"}</div>`;
+            const addrTotal = formatAddressTotal(getAddressValue(addr));
+            html += `<div class="text-xs text-muted text-right min-h-[1rem]">${addrTotal || "&nbsp;"}</div>`;
             html += balanceLinesForAddress(
                 addr,
                 state.trackedTokens,

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,
 };