diff --git a/README.md b/README.md index 8ae51e1..fcfc9af 100644 --- a/README.md +++ b/README.md @@ -1396,7 +1396,8 @@ lists, the send-screen token selector, the address total and the remove-address warning (`isBelowOneMillionth()` in `src/shared/amountDisplay.js`). The Send and confirmation screens show it when its token is the one being sent. Tracked tokens with a zero balance are listed as well while "Show tracked tokens with -zero balance" is on. +zero balance" is on; a token just added on AddToken whose balance no refresh has +fetched yet is listed there as `quantity unknown` (see AddToken). #### Stored state and its version @@ -1567,6 +1568,13 @@ such as an edit typed into Settings and not yet saved. Home is the one screen "Back" always re-renders, so the wallet list reflects anything that changed while the user was away from it. +The popup refreshes balances every ten seconds while it is open +(`doRefreshAndRender()` in `src/popup/index.js`). Each refresh that returns +draws the wallet list again and, when AddressDetail or AddressToken is up, that +screen's USD total and balance lines; its transaction list is left alone. No +other screen is drawn again by a refresh, since the rest show no balances or +hold what the user typed. + A network switch leaves every screen on the page showing the previous network. The user makes it in Settings, or approves a site's request and the background saves it. Any other popup open meanwhile takes the switch from the stored @@ -2078,7 +2086,13 @@ view would leave a wallet one click from deletion. - "Add" button - **Transitions**: - "Add" (valid contract) → tracks the token, pops the stack, and re-renders - **AddressDetail** + **AddressDetail**. The token's balance is not known until a balance + refresh that started with it tracked has fetched every address's token + balances; until then its balance line reads `quantity unknown`, never + `0.0000`, unless the explorer had already listed a holding of it. A + refresh that returns draws AddressDetail's balances again (see + Navigation), so the fetched balance shows without the screen being + reopened - "Add" with a token already tracked, a scam-listed address, or a failed contract lookup → flash message, no screen change - "Add", then a network switch in Settings before the contract lookup diff --git a/src/popup/index.js b/src/popup/index.js index f05a031..4ce4139 100644 --- a/src/popup/index.js +++ b/src/popup/index.js @@ -21,6 +21,7 @@ const { showSaveFailureBanner, pushCurrentView, goBack, + tokensNotFetched, } = require("./views/helpers"); const { applyTheme } = require("./theme"); const { @@ -82,12 +83,27 @@ let refreshTimer = null; // (https://git.eeqj.de/sneak/AutistMask/issues/577). A dropped refresh is not // recorded as done. Results are copied by address, so an address added or // removed meanwhile is left as it is. +// +// Once the save is done, the wallet list is drawn again, and so are the +// balances on AddressDetail or AddressToken when that is the screen up. No +// other screen is drawn again, since the rest show no balances or hold what the +// user typed. async function doRefreshAndRender() { if (refreshInFlight) return; refreshInFlight = true; try { const networkId = state.networkId; + // The tracked tokens this refresh asks the explorer about. A token + // added on AddToken after this point is not among them, so this + // refresh does not count as having fetched it. + const tokens = state.trackedTokens.map((t) => t.address.toLowerCase()); const wallets = structuredClone(state.wallets); + // Taken off the copy, so that an address whose token balances the + // explorer did not return comes back without any and keeps its + // stored ones. + for (const wallet of wallets) { + for (const addr of wallet.addresses) delete addr.tokenBalances; + } await Promise.all([ refreshPrices(), refreshBalances( @@ -100,9 +116,13 @@ async function doRefreshAndRender() { ), ]); const refreshed = new Map(); + let allTokenBalancesFetched = true; for (const wallet of wallets) { for (const addr of wallet.addresses) { refreshed.set(addr.address, addr); + if (addr.tokenBalances === undefined) { + allTokenBalancesFetched = false; + } } } await saveState((stored) => { @@ -117,10 +137,17 @@ async function doRefreshAndRender() { } } } + // Each token this refresh asked about is now fetched for every + // address. + if (allTokenBalancesFetched) { + for (const token of tokens) tokensNotFetched.delete(token); + } state.lastBalanceRefresh = Date.now(); return true; }); renderWalletList(); + if (state.currentView === "address") addressDetail.renderBalances(); + if (state.currentView === "address-token") addressToken.renderBalance(); } catch (e) { // Every call site fires this and walks away — the boot below, the ten // second interval, and eight views through ctx — so it must never diff --git a/src/popup/views/addToken.js b/src/popup/views/addToken.js index 76c96d0..fc10b09 100644 --- a/src/popup/views/addToken.js +++ b/src/popup/views/addToken.js @@ -1,4 +1,11 @@ -const { $, showView, showFlash, escapeHtml, goBack } = require("./helpers"); +const { + $, + showView, + showFlash, + escapeHtml, + goBack, + tokensNotFetched, +} = require("./helpers"); const { getTopTokens } = require("../../shared/tokenList"); const { state, saveState, currentNetwork } = require("../../shared/state"); const { lookupTokenInfo } = require("../../shared/balances"); @@ -78,6 +85,7 @@ function init(ctx) { decimals: info.decimals, name: info.name, }); + tokensNotFetched.add(contractAddr.toLowerCase()); await saveState(); ctx.doRefreshAndRender(); // Pop the stack (back to address detail) and re-render it diff --git a/src/popup/views/addressDetail.js b/src/popup/views/addressDetail.js index fa6646e..58838d0 100644 --- a/src/popup/views/addressDetail.js +++ b/src/popup/views/addressDetail.js @@ -65,11 +65,25 @@ function show() { }); $("address-line").dataset.full = addr.address; attachCopyHandlers($("address-line")); - const usdTotal = formatAddressTotal(getAddressValue(addr)); - $("address-usd-total").innerHTML = escapeHtml(usdTotal) || " "; const ensEl = $("address-ens"); // ENS is now shown inside renderAddressHtml, hide the separate element ensEl.classList.add("hidden"); + renderBalances(); + renderSendTokenSelect(addr); + $("tx-list").innerHTML = + '
Loading...
'; + showView("address"); + loadTransactions(addr.address); +} + +// The USD total and the balance list. Drawn again by each balance refresh +// that returns while this screen is up (doRefreshAndRender() in +// src/popup/index.js), which leaves the rest of the screen as it is. +function renderBalances() { + const addr = + state.wallets[state.selectedWallet].addresses[state.selectedAddress]; + const usdTotal = formatAddressTotal(getAddressValue(addr)); + $("address-usd-total").innerHTML = escapeHtml(usdTotal) || " "; $("address-balances").innerHTML = balanceLinesForAddress( addr, state.trackedTokens, @@ -83,11 +97,6 @@ function show() { ctx.showAddressToken(); }); }); - renderSendTokenSelect(addr); - $("tx-list").innerHTML = - '
Loading...
'; - showView("address"); - loadTransactions(addr.address); } let loadedTxs = []; @@ -280,4 +289,4 @@ function init(_ctx) { exportPrivkey.init(); } -module.exports = { init, show }; +module.exports = { init, show, renderBalances }; diff --git a/src/popup/views/addressToken.js b/src/popup/views/addressToken.js index 207d0fb..4ade4fc 100644 --- a/src/popup/views/addressToken.js +++ b/src/popup/views/addressToken.js @@ -13,6 +13,7 @@ const { nativeCurrency, balanceLine, unknownableAmount, + tokensNotFetched, renderAddressHtml, attachCopyHandlers, goBack, @@ -48,37 +49,18 @@ function show() { const addr = wallet.addresses[state.selectedAddress]; const ai = state.selectedAddress; const tokenId = state.selectedToken; - - // Determine token symbol and balance - let symbol, amount, price; const knownToken = TOKEN_BY_ADDRESS.get(tokenId.toLowerCase()); - if (tokenId === "ETH") { - symbol = nativeCurrency(); - amount = parseFloat(addr.balance || "0"); - price = getPrice("ETH"); - } else { - const tb = (addr.tokenBalances || []).find( - (t) => t.address.toLowerCase() === tokenId.toLowerCase(), - ); - symbol = resolveSymbol( - tokenId, - addr.tokenBalances, - state.trackedTokens, - ); - // null when the scale is unknown: no quantity to show, and none to - // price. balanceLine() states that rather than printing 0.0000. - amount = tb ? unknownableAmount(tb.balance) : 0; - price = getPrice(symbol); - } - - currentSymbol = symbol; + currentSymbol = + tokenId === "ETH" + ? nativeCurrency() + : resolveSymbol(tokenId, addr.tokenBalances, state.trackedTokens); $("address-token-title").textContent = wallet.name + " \u2014 Address " + (ai + 1) + " \u2014 " + - displaySymbol(symbol); + displaySymbol(currentSymbol); // Blockie const blockieEl = $("address-token-jazzicon"); @@ -100,13 +82,7 @@ function show() { $("address-token-line").dataset.full = addr.address; attachCopyHandlers($("address-token-line")); - // USD total for this token only - const usdVal = price && amount !== null ? amount * price : null; - const usdStr = formatUsd(usdVal); - $("address-token-usd-total").innerHTML = escapeHtml(usdStr) || " "; - - // Single token balance line (no tokenId — not clickable here) - $("address-token-balance").innerHTML = balanceLine(symbol, amount, price); + renderBalance(); // Token contract details (ERC-20 only) const contractInfo = $("address-token-contract-info"); @@ -168,6 +144,44 @@ function show() { loadTransactions(addr.address, tokenId); } +// The USD total and the balance line of the selected token. Drawn again by +// each balance refresh that returns while this screen is up +// (doRefreshAndRender() in src/popup/index.js), which leaves the rest of the +// screen as it is. +function renderBalance() { + const addr = + state.wallets[state.selectedWallet].addresses[state.selectedAddress]; + const tokenId = state.selectedToken; + let amount, price; + if (tokenId === "ETH") { + amount = parseFloat(addr.balance || "0"); + price = getPrice("ETH"); + } else { + const id = tokenId.toLowerCase(); + const tb = (addr.tokenBalances || []).find( + (t) => t.address.toLowerCase() === id, + ); + // null when the scale is unknown, or when no refresh has fetched a + // token just added: no quantity to show, and none to price. + // balanceLine() states that rather than printing 0.0000. + if (tb) amount = unknownableAmount(tb.balance); + else amount = tokensNotFetched.has(id) ? null : 0; + price = getPrice(currentSymbol); + } + + // USD total for this token only + const usdVal = price && amount !== null ? amount * price : null; + const usdStr = formatUsd(usdVal); + $("address-token-usd-total").innerHTML = escapeHtml(usdStr) || " "; + + // Single token balance line (no tokenId — not clickable here) + $("address-token-balance").innerHTML = balanceLine( + currentSymbol, + amount, + price, + ); +} + async function loadTransactions(address, tokenId) { try { const rawTxs = await fetchRecentTransactions( @@ -343,4 +357,4 @@ function init(_ctx) { }); } -module.exports = { init, show }; +module.exports = { init, show, renderBalance }; diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index cd9fa8d..f6bb360 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -380,6 +380,15 @@ function balanceLine(symbol, amount, price, tokenId) { ); } +// Contract addresses, lowercased, of the tokens added on AddToken that no +// balance refresh has fetched yet. An address with no balance row for one of +// them has no known balance of it, so its line says the quantity is unknown +// rather than 0.0000. AddToken adds a token here, and doRefreshAndRender() in +// src/popup/index.js takes it off once a refresh that started with it tracked +// has fetched every address's token balances +// (https://git.eeqj.de/sneak/AutistMask/issues/516). +const tokensNotFetched = new Set(); + function balanceLinesForAddress(addr, trackedTokens, showZero) { let html = balanceLine( nativeCurrency(), @@ -409,12 +418,13 @@ function balanceLinesForAddress(addr, trackedTokens, showZero) { } if (showZero && trackedTokens) { for (const t of trackedTokens) { - if (seen.has(t.address.toLowerCase())) continue; + const id = t.address.toLowerCase(); + if (seen.has(id)) continue; html += balanceLine( t.symbol, - 0, + tokensNotFetched.has(id) ? null : 0, getPrice(t.symbol), - t.address.toLowerCase(), + id, ); } } @@ -786,6 +796,7 @@ module.exports = { flashCopyFeedback, balanceLine, balanceLinesForAddress, + tokensNotFetched, addressHoldsFunds, keyMaterialName, unknownableAmount, diff --git a/tests/addedTokenBalance.test.js b/tests/addedTokenBalance.test.js new file mode 100644 index 0000000..1e96b9e --- /dev/null +++ b/tests/addedTokenBalance.test.js @@ -0,0 +1,204 @@ +// After "Add" on AddToken, AddressDetail lists the new token, and its line reads +// `quantity unknown`, not 0.0000, until a balance refresh that asked the +// explorer about the token has returned for every address. A refresh that +// returns draws the balances on AddressDetail or AddressToken again, so the +// fetched balance shows without the screen being reopened +// (https://git.eeqj.de/sneak/AutistMask/issues/516). Driven through the real +// popup over a stored profile on Ethereum Mainnet, with "Show tracked tokens +// with zero balance" on, as it is by default. + +const { + bootPopup, + cleanupPopup, + unversionedValidProfile, +} = require("./support/popupBoot"); + +// Tracked by nobody until a test adds it. +const NEW_TOKEN = "0xBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; + +// The explorer's row for a holding of a million NEW_TOKEN, in the shape +// fetchTokenBalances() stores it. +const MILLION = { + address: NEW_TOKEN, + name: "New Token", + symbol: "NEW", + decimals: 18, + balance: "1000000.0", + holders: 3, +}; + +afterEach(() => { + cleanupPopup(); +}); + +// The popup reopened onto AddressDetail for the one address. +function bootOnAddress() { + return bootPopup( + unversionedValidProfile({ + currentView: "address", + selectedWallet: 0, + selectedAddress: 0, + }), + ); +} + +// Holds the next balance refresh open. The returned function lets it finish +// with the address's ETH balance at 2 and `tokenBalances` as the explorer's +// rows for it, or with no token balances at all when given null: an explorer +// that did not answer, which the real refreshBalances() leaves unwritten. +function holdNextRefresh(page) { + const { refreshBalances } = require("../src/shared/balances"); + let release; + refreshBalances.mockImplementationOnce(async (wallets) => { + const tokenBalances = await new Promise((resolve) => { + release = resolve; + }); + wallets[0].addresses[0].balance = "2.0"; + if (tokenBalances !== null) { + wallets[0].addresses[0].tokenBalances = tokenBalances; + } + }); + return async (tokenBalances) => { + release(tokenBalances); + await page.settle(); + }; +} + +// "+ Token", NEW_TOKEN typed in, and "Add", with the contract lookup +// answering at once. +async function addNewToken(page) { + const { lookupTokenInfo } = require("../src/shared/balances"); + lookupTokenInfo.mockResolvedValueOnce({ + symbol: "NEW", + decimals: 18, + name: "New Token", + }); + await page.click("btn-add-token"); + page.node("add-token-address").value = NEW_TOKEN; + await page.click("btn-add-token-confirm"); +} + +// NEW_TOKEN's row in AddressDetail's balance list, as markup. +function newTokenRow(page) { + return page + .node("address-balances") + .innerHTML.split("
+ row.includes('data-token="' + NEW_TOKEN.toLowerCase() + '"'), + ); +} + +// AddressToken for `token` opened from AddressDetail, as tapping the token's +// row there does. +async function openToken(page, token) { + const { state } = require("../src/shared/state"); + state.selectedToken = token; + require("../src/popup/views/helpers").pushCurrentView(); + require("../src/popup/views/addressToken").show(); + await page.settle(); +} + +test("until the refresh Add started returns, the new token's line reads quantity unknown", async () => { + const page = await bootOnAddress(); + expect(page.pageErrors).toEqual([]); + const finish = holdNextRefresh(page); + + await addNewToken(page); + + expect(page.visibleViews()).toEqual(["address"]); + const row = newTokenRow(page); + expect(row).toContain("quantity unknown"); + expect(row).not.toContain("0.0000"); + await finish([MILLION]); +}); + +test("once that refresh returns, AddressDetail shows the fetched balance without being reopened", async () => { + const page = await bootOnAddress(); + const finish = holdNextRefresh(page); + await addNewToken(page); + // Stands for the transactions on the screen, which only show() replaces. + page.node("tx-list").innerHTML = "transactions on the screen"; + + await finish([MILLION]); + + expect(page.visibleViews()).toEqual(["address"]); + expect(newTokenRow(page)).toContain("1000000.0000"); + // Only the balances are drawn again, not the transaction list. + expect(page.node("tx-list").innerHTML).toBe("transactions on the screen"); +}); + +test("a refresh already running when the token is added does not count as fetching it", async () => { + const page = await bootOnAddress(); + const finishRunning = holdNextRefresh(page); + const ticking = page.tick(); + await page.settle(); + + await addNewToken(page); + await finishRunning([]); + await ticking; + + expect(newTokenRow(page)).toContain("quantity unknown"); + + // The next one asks the explorer about the token, which lists no holding + // of it: a fetched zero. + const finishNext = holdNextRefresh(page); + const next = page.tick(); + await page.settle(); + await finishNext([]); + await next; + + expect(newTokenRow(page)).toContain("0.0000"); +}); + +test("a refresh in which the explorer did not answer leaves the new token's line quantity unknown", async () => { + const page = await bootOnAddress(); + const finish = holdNextRefresh(page); + await addNewToken(page); + + await finish(null); + + // Drawn again: the ETH balance it fetched is shown. + expect(page.node("address-balances").innerHTML).toContain("2.0000"); + expect(newTokenRow(page)).toContain("quantity unknown"); +}); + +test("AddressToken for the new token reads quantity unknown until the refresh returns, then the fetched balance", async () => { + const page = await bootOnAddress(); + const finish = holdNextRefresh(page); + await addNewToken(page); + + await openToken(page, NEW_TOKEN.toLowerCase()); + + expect(page.visibleViews()).toEqual(["address-token"]); + const before = page.node("address-token-balance").innerHTML; + expect(before).toContain("quantity unknown"); + expect(before).not.toContain("0.0000"); + + await finish([MILLION]); + + expect(page.node("address-token-balance").innerHTML).toContain( + "1000000.0000", + ); +}); + +test("a refresh that returns while AddressToken is up draws its balance again", async () => { + const page = await bootPopup( + unversionedValidProfile({ + currentView: "address-token", + viewStack: ["main", "address"], + selectedWallet: 0, + selectedAddress: 0, + selectedToken: "ETH", + }), + ); + expect(page.visibleViews()).toEqual(["address-token"]); + expect(page.node("address-token-balance").innerHTML).toContain("1.5000"); + const finish = holdNextRefresh(page); + const ticking = page.tick(); + await page.settle(); + + await finish(null); + await ticking; + + expect(page.node("address-token-balance").innerHTML).toContain("2.0000"); +}); diff --git a/tests/e2e/run.js b/tests/e2e/run.js index 2d95c8f..c560346 100644 --- a/tests/e2e/run.js +++ b/tests/e2e/run.js @@ -651,7 +651,8 @@ test("a token added by its contract address is listed on the address screen (#15 await visible(env.page, "#view-address"); // No wait: the confirm renders the balance list before it shows the - // screen, and nothing renders the list again while the screen is up. + // screen. The refresh it starts draws the list again, with the row still + // in it. const row = env.page.locator( '#address-balances [data-token="' + STUB_TOKEN.address + '"]', { hasText: STUB_TOKEN.symbol }, diff --git a/tests/flashLine.test.js b/tests/flashLine.test.js index ce00781..36b5625 100644 --- a/tests/flashLine.test.js +++ b/tests/flashLine.test.js @@ -88,6 +88,7 @@ describe.each([ showFlash: (msg) => flashes.push(msg), escapeHtml: (s) => s, goBack: () => {}, + tokensNotFetched: new Set(), })); require("../src/popup/views/" + view).init({