diff --git a/README.md b/README.md index eccafec..e39d2f1 100644 --- a/README.md +++ b/README.md @@ -1197,14 +1197,23 @@ The token's symbol comes from the same places. The bundled list answers for a token on Ethereum Mainnet only. When none of them answers, the amount reads `5000000000 base units (decimals unknown)`, and nothing is formatted with guessed decimals. A holding whose decimals are unknown shows "quantity unknown" -in the balance lists. A token just added shows "quantity unknown" in place of a -zero, until a refresh has read its balance at every address. Only a whole number -from 0 to 80 counts as a token's decimals. A token's `decimals` is stored only -as a source reported it, never guessed. A `null` balance means the decimals are -unknown, never that the balance is zero. While "Use Blockscout" is off, the -bundled list and the tracked tokens may lack a token's symbol or decimals. The -approval and status screens then add a line under that token saying that -Blockscout's report is not used. +in the balance lists. + +A tracked token with no stored balance at an address can show "quantity unknown" +there in place of a zero. It does so after each of these: + +- the token is added; +- an address or a wallet is added; +- the network is switched. + +This lasts until a refresh has read the token's balance at every address. + +Only a whole number from 0 to 80 counts as a token's decimals. A token's +`decimals` is stored only as a source reported it, never guessed. A `null` +balance means the decimals are unknown, never that the balance is zero. While +"Use Blockscout" is off, the bundled list and the tracked tokens may lack a +token's symbol or decimals. The approval and status screens then add a line +under that token saying that Blockscout's report is not used. The wallet's own ERC-20 send is encoded with the decimals the screen showed. At signing time the contract's `decimals()` is read only to be compared with those. diff --git a/src/popup/index.js b/src/popup/index.js index f4db425..dce17b2 100644 --- a/src/popup/index.js +++ b/src/popup/index.js @@ -98,11 +98,11 @@ let refreshTimer = null; // not tracked if the stored setting is off by then. // // The save also deletes `balanceNotFetched` from each tracked token whose -// balance this refresh read at every address. trackToken() in -// src/popup/views/helpers.js sets that field. The save takes the field off the -// matching tokens of the stored record (clearBalanceNotFetched() in -// src/shared/state.js). The rest of the stored tracked token list stays as it -// is. +// balance this refresh read at every address. Adding a token sets that field +// (trackToken() in src/popup/views/helpers.js). So does adding an address, and +// so does a network switch. The save takes the field off the matching tokens of +// the stored record (clearBalanceNotFetched() in src/shared/state.js). The rest +// of the stored tracked token list stays as it is. // // Once the save is done, the wallet list is drawn again. So are the balances // on AddressDetail or AddressToken when that screen is up. No other screen is diff --git a/src/popup/views/addWallet.js b/src/popup/views/addWallet.js index b5629b5..abdc485 100644 --- a/src/popup/views/addWallet.js +++ b/src/popup/views/addWallet.js @@ -7,6 +7,7 @@ const { goBack, clearViewStack, onViewLeave, + markTrackedTokensNotFetched, } = require("./helpers"); const { generateMnemonic, @@ -195,6 +196,7 @@ async function importMnemonic(ctx) { ], }; state.wallets.push(wallet); + markTrackedTokensNotFetched(); state.hasWallet = true; if (state.activeAddress === null) { state.activeAddress = firstAddress; @@ -220,6 +222,7 @@ async function importMnemonic(ctx) { tokenBalances: [], })); wallet.nextIndex = scan.nextIndex; + markTrackedTokensNotFetched(); // The new list leaves out the first address when the scan found it // unused. If it was the active one, the new list's first takes over, // and the background is told as when the user picks one on Home. @@ -266,6 +269,7 @@ async function importPrivateKey(ctx) { encryptedSecret: encrypted, addresses: [{ address: addr, balance: "0.0000", tokenBalances: [] }], }); + markTrackedTokensNotFetched(); state.hasWallet = true; if (state.activeAddress === null) { state.activeAddress = addr; @@ -330,6 +334,7 @@ async function importXprvKey(ctx) { ], }; state.wallets.push(wallet); + markTrackedTokensNotFetched(); state.hasWallet = true; if (state.activeAddress === null) { state.activeAddress = firstAddress; @@ -355,6 +360,7 @@ async function importXprvKey(ctx) { tokenBalances: [], })); wallet.nextIndex = scan.nextIndex; + markTrackedTokensNotFetched(); // As for a recovery phrase: an active first address the scan left // out gives way to the new list's first. const activeMoved = diff --git a/src/popup/views/addressToken.js b/src/popup/views/addressToken.js index e815ede..42234a9 100644 --- a/src/popup/views/addressToken.js +++ b/src/popup/views/addressToken.js @@ -172,8 +172,8 @@ function renderBalance() { (t) => t.address.toLowerCase() === id, ); // null when the scale is unknown, or when no refresh has read the - // balance of a token just added: no quantity to show, and none to - // price. balanceLine() states that rather than printing 0.0000. + // token's balance: no quantity to show, and none to price. + // balanceLine() states that rather than printing 0.0000. if (tb) amount = unknownableAmount(tb.balance); else amount = tracked && tracked.balanceNotFetched ? null : 0; price = getPrice(tokenId); diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index 0be0ea0..9198ee0 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -430,6 +430,13 @@ function trackToken(token) { state.trackedTokens.push({ ...token, balanceNotFetched: true }); } +// Gives every tracked token `balanceNotFetched: true`, as trackToken() gives a +// new one. Called when an address is added, before the save: no refresh has +// read any token's balance at that address. +function markTrackedTokensNotFetched() { + for (const token of state.trackedTokens) token.balanceNotFetched = true; +} + function balanceLinesForAddress(addr, trackedTokens, showZero) { let html = balanceLine( nativeCurrency(), @@ -850,6 +857,7 @@ module.exports = { balanceLine, balanceLinesForAddress, trackToken, + markTrackedTokensNotFetched, addressHoldsFunds, keyMaterialName, unknownableAmount, diff --git a/src/popup/views/home.js b/src/popup/views/home.js index 43a76e2..d9fea96 100644 --- a/src/popup/views/home.js +++ b/src/popup/views/home.js @@ -3,6 +3,7 @@ const { showView, showFlash, balanceLinesForAddress, + markTrackedTokensNotFetched, isoDate, timeAgo, addressDotHtml, @@ -394,6 +395,7 @@ function render(ctx) { tokenBalances: [], }); wallet.nextIndex++; + markTrackedTokensNotFetched(); await saveState(); render(ctx); ctx.doRefreshAndRender(); diff --git a/src/popup/views/send.js b/src/popup/views/send.js index 37bfe43..b8f3301 100644 --- a/src/popup/views/send.js +++ b/src/popup/views/send.js @@ -242,10 +242,11 @@ function updateSendBalance() { state.trackedTokens, currentNetwork().chainId, ); - // A null balance is a holding whose scale is unknown. Saying a figure - // for it would be a claim about the amount, so it reads as the - // confirmation screen's balance line reads it; the send itself is - // refused later by transferAmountUnits() for the same missing scale. + // A null balance is a holding whose scale is unknown, or a tracked + // token whose balance no refresh has read. Saying a figure for it would + // be a claim about the amount, so it reads as the confirmation screen's + // balance line reads it. transferAmountUnits() later refuses a send at + // a missing scale. const bal = tokenBalanceAndDecimals(addr, token).tokenBalance; $("send-balance").textContent = bal == null diff --git a/src/shared/chainSwitchFields.js b/src/shared/chainSwitchFields.js index 356f039..7314a71 100644 --- a/src/shared/chainSwitchFields.js +++ b/src/shared/chainSwitchFields.js @@ -48,9 +48,14 @@ function applyChainSwitchFields(s, newNetworkId) { // It is a copy because a save writes a changed s.networkTrackedTokens whole, // over any list another page stored. An add or removal on the active network // must change only s.trackedTokens. + // + // Each token of the copy gets `balanceNotFetched`, because the balances are + // cleared below and no refresh has read this network's yet. Each reads + // `quantity unknown` until one has, not 0.0000. if (!s.networkTrackedTokens) s.networkTrackedTokens = {}; s.networkTrackedTokens[s.networkId] = s.trackedTokens; s.trackedTokens = structuredClone(s.networkTrackedTokens[net.id] || []); + for (const token of s.trackedTokens) token.balanceNotFetched = true; s.networkId = net.id; s.rpcUrl = remembered.rpcUrl || net.defaultRpcUrl; diff --git a/tests/addedTokenBalance.test.js b/tests/addedTokenBalance.test.js index c3ef97b..0e4bf70 100644 --- a/tests/addedTokenBalance.test.js +++ b/tests/addedTokenBalance.test.js @@ -11,14 +11,44 @@ // switch keeps each network's records apart. A refresh takes the field off the // stored record's tokens and keeps another page's changes to the stored list. // +// Every tracked token carries that field again once an address or a wallet is +// added, and after a network switch, so it reads the same until a refresh has +// read it (https://git.eeqj.de/sneak/AutistMask/issues/720). +// // 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 { HDNodeWallet, Mnemonic } = require("ethers"); + const { bootPopup, cleanupPopup, unversionedValidProfile, } = require("./support/popupBoot"); +const { hdWalletFromMnemonic } = require("../src/shared/wallet"); + +const PHRASE = + "abandon abandon abandon abandon abandon abandon " + + "abandon abandon abandon abandon abandon about"; +const PASSWORD = "correct horse battery staple"; + +// What AddWallet's tabs take. The first and last scan for more addresses. +const FROM_PHRASE = { + tab: "tab-mnemonic", + field: "wallet-mnemonic", + secret: PHRASE, +}; +const FROM_PRIVATE_KEY = { + tab: "tab-privkey", + field: "import-private-key", + secret: "0x" + "11".repeat(32), +}; +const FROM_XPRV = { + tab: "tab-xprv", + field: "import-xprv-key", + secret: HDNodeWallet.fromSeed(Mnemonic.fromPhrase(PHRASE).computeSeed()) + .extendedKey, +}; // Tracked by nobody until a test adds it. const NEW_TOKEN = "0xBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; @@ -175,6 +205,47 @@ function newTokenRow(page, id = "address-balances") { ); } +// Every address's row for NEW_TOKEN on Home, as markup, in the order Home lists +// the addresses. +function newTokenRowsOnHome(page) { + return page + .node("wallet-list") + .innerHTML.split("
+ row.includes('data-token="' + NEW_TOKEN.toLowerCase() + '"'), + ); +} + +// The "+" button Home draws for the first wallet. As in firstRemoveButton(), +// the list hands Home a stand-in for that button each time it is drawn, and +// `press()` presses the last one drawn. Call it before Home next draws its list. +function plusButton(page) { + const button = { press: null }; + page.node("wallet-list").querySelectorAll = (selector) => + selector !== ".btn-add-address" + ? [] + : [ + { + dataset: { wallet: "0" }, + addEventListener: (name, fn) => { + button.press = () => + fn({ stopPropagation: () => {} }); + }, + }, + ]; + return button; +} + +// "Add wallet" on Home, filled in from `source`, and "Import". +async function addWallet(page, { tab, field, secret }) { + await page.click("btn-add-wallet-bottom"); + await page.click(tab); + page.node(field).value = secret; + page.node("add-wallet-password").value = PASSWORD; + page.node("add-wallet-password-confirm").value = PASSWORD; + await page.click("btn-add-wallet-confirm"); +} + // AddressDetail for the one address opened from Home, as its `[info]` button // does. async function openAddress(page) { @@ -456,3 +527,143 @@ test("a refresh does not bring back a token that another page removed after this NEW_TRACKED, ]); }); + +test("an address just added reads quantity unknown for a tracked token until a refresh has read it", async () => { + const { xpub, firstAddress } = hdWalletFromMnemonic(PHRASE); + const page = await bootPopup( + unversionedValidProfile({ + wallets: [ + { + type: "hd", + name: "Wallet 1", + xpub, + encryptedSecret: "encrypted-secret-1", + nextIndex: 1, + addresses: [ + { + address: firstAddress, + balance: "1.5", + tokenBalances: [MILLION], + }, + ], + }, + ], + activeAddress: firstAddress, + trackedTokens: [NEW_TRACKED], + }), + ); + const plus = plusButton(page); + await refreshOnce(page, [MILLION]); + const finish = holdNextRefresh(page); + + await plus.press(); + await page.settle(); + + const [held, added] = newTokenRowsOnHome(page); + expect(held).toContain("1000000.0000"); + expect(added).toContain("quantity unknown"); + expect(added).not.toContain("0.0000"); + expect( + page.storage.read("autistmask").trackedTokens[0].balanceNotFetched, + ).toBe(true); + + // It reads the new address's balance, which is zero. + await finish([MILLION]); + + expect(newTokenRowsOnHome(page)[1]).toContain("0.0000"); +}); + +test.each([FROM_PHRASE, FROM_PRIVATE_KEY, FROM_XPRV])( + "a wallet just added from $tab reads quantity unknown for a tracked token until a refresh has read it", + async (source) => { + const page = await bootPopup( + unversionedValidProfile({ trackedTokens: [NEW_TRACKED] }), + ); + const finish = holdNextRefresh(page); + + await addWallet(page, source); + + const added = newTokenRowsOnHome(page)[1]; + expect(added).toContain("quantity unknown"); + expect(added).not.toContain("0.0000"); + + // It reads the new address's balance, which is zero. + await finish([]); + + expect(newTokenRowsOnHome(page)[1]).toContain("0.0000"); + }, +); + +test.each([FROM_PHRASE, FROM_XPRV])( + "addresses the scan finds for a wallet from $tab read quantity unknown, even after a refresh returned during the scan", + async (source) => { + const page = await bootPopup( + unversionedValidProfile({ trackedTokens: [NEW_TRACKED] }), + ); + const { scanForAddresses } = require("../src/shared/balances"); + let finish; + scanForAddresses.mockImplementationOnce(async () => { + await refreshOnce(page, []); + finish = holdNextRefresh(page); + return { + addresses: [ + { address: "0x" + "dd".repeat(20), index: 0 }, + { address: "0x" + "ee".repeat(20), index: 1 }, + ], + nextIndex: 2, + }; + }); + + await addWallet(page, source); + + const [, first, second] = newTokenRowsOnHome(page); + for (const found of [first, second]) { + expect(found).toContain("quantity unknown"); + expect(found).not.toContain("0.0000"); + } + + await finish([]); + + expect(newTokenRowsOnHome(page)[2]).toContain("0.0000"); + }, +); + +test("after a network switch in Settings, a tracked token reads quantity unknown until a refresh has read it", async () => { + const page = await bootOnAddress({ + trackedTokens: [OTHER_TOKEN], + networkTrackedTokens: { sepolia: [NEW_TRACKED] }, + }); + + await page.click("btn-settings"); + await chooseNetwork(page, "sepolia"); + await page.click("btn-settings-back"); + + expect(page.visibleViews()).toEqual(["address"]); + const row = newTokenRow(page); + expect(row).toContain("quantity unknown"); + expect(row).not.toContain("0.0000"); + + // It reads the balance, which is zero. + await refreshOnce(page, []); + + expect(newTokenRow(page)).toContain("0.0000"); +}); + +test("after a switch saved by the background, a tracked token reads quantity unknown until a refresh has read it", async () => { + const page = await bootOnAddress({ + networkTrackedTokens: { sepolia: [NEW_TRACKED] }, + }); + const { + applyChainSwitchFields, + } = require("../src/shared/chainSwitchFields"); + const record = page.storage.read("autistmask"); + applyChainSwitchFields(record, "sepolia"); + page.storage.write("autistmask", record); + + // The popup takes the switch, and this refresh reads no token's balance. + await page.tick(); + + const row = newTokenRow(page); + expect(row).toContain("quantity unknown"); + expect(row).not.toContain("0.0000"); +}); diff --git a/tests/addressScanCancelled.test.js b/tests/addressScanCancelled.test.js index 48815d0..bff6c24 100644 --- a/tests/addressScanCancelled.test.js +++ b/tests/addressScanCancelled.test.js @@ -44,6 +44,7 @@ jest.doMock("../src/popup/views/helpers", () => ({ goBack: () => {}, clearViewStack: () => {}, onViewLeave: () => {}, + markTrackedTokensNotFetched: () => {}, })); // state.js reads chrome.storage.local at load, and saveState() writes it. diff --git a/tests/coldWorkerChainSwitch.test.js b/tests/coldWorkerChainSwitch.test.js index 1ed03e1..a89a2f7 100644 --- a/tests/coldWorkerChainSwitch.test.js +++ b/tests/coldWorkerChainSwitch.test.js @@ -242,7 +242,12 @@ describe("a chain switch on a worker that never loaded state", () => { expect(bg.persisted().blockscoutUrl).toBe(CUSTOM_BLOCKSCOUT); expect(bg.persisted().wallets).toEqual(walletFixture()); expect(bg.persisted().trackedTokens).toEqual([ - { address: TOKEN, symbol: "DAI", decimals: 18 }, + { + address: TOKEN, + symbol: "DAI", + decimals: 18, + balanceNotFetched: true, + }, ]); }); diff --git a/tests/popupFollowsNetworkSwitch.test.js b/tests/popupFollowsNetworkSwitch.test.js index 1c37373..a82bb39 100644 --- a/tests/popupFollowsNetworkSwitch.test.js +++ b/tests/popupFollowsNetworkSwitch.test.js @@ -245,7 +245,9 @@ test("a switch back in Settings after a follow whose write failed is stored", as const stored = page.storage.read("autistmask"); expect(stored.networkId).toBe("mainnet"); expect(stored.rpcUrl).toBe(NETWORKS.mainnet.defaultRpcUrl); - expect(stored.trackedTokens).toEqual([MAINNET_TOKEN]); + expect(stored.trackedTokens).toEqual([ + { ...MAINNET_TOKEN, balanceNotFetched: true }, + ]); expect(page.text("flash-msg")).toBe("Switched to Ethereum Mainnet."); const told = globalThis.chrome.runtime.sendMessage.mock.calls .map(([message]) => message) diff --git a/tests/trackedTokensPerNetwork.test.js b/tests/trackedTokensPerNetwork.test.js index 77c6877..62212c4 100644 --- a/tests/trackedTokensPerNetwork.test.js +++ b/tests/trackedTokensPerNetwork.test.js @@ -180,7 +180,10 @@ test("a token added by contract address on Sepolia is tracked on Sepolia only", await boot.click("btn-settings"); await chooseNetwork(boot, "mainnet"); expect(stored().networkId).toBe("mainnet"); - expect(stored().trackedTokens).toEqual([MAINNET_TOKEN]); + // No refresh has read a balance on this network since the switch. + expect(stored().trackedTokens).toEqual([ + { ...MAINNET_TOKEN, balanceNotFetched: true }, + ]); const onMainnet = boot.node("settings-tracked-tokens").innerHTML; expect(onMainnet).toContain("DAI"); expect(onMainnet).not.toContain("TST"); @@ -276,7 +279,9 @@ test("a token tracked on Sepolia when another page switched there is not tracked const stored = boot.storage.read("autistmask"); expect(stored.networkId).toBe("sepolia"); - expect(stored.trackedTokens).toEqual([SEPOLIA_TOKEN]); + expect(stored.trackedTokens).toEqual([ + { ...SEPOLIA_TOKEN, balanceNotFetched: true }, + ]); expect(stored.networkTrackedTokens.mainnet).toEqual([]); }); @@ -441,7 +446,10 @@ test("a token another page added on Ethereum Mainnet is tracked when this page s const stored = boot.storage.read("autistmask"); expect(stored.networkId).toBe("mainnet"); - expect(stored.trackedTokens).toEqual([MAINNET_TOKEN, newToken]); + expect(stored.trackedTokens).toEqual([ + { ...MAINNET_TOKEN, balanceNotFetched: true }, + { ...newToken, balanceNotFetched: true }, + ]); }); // This page tracks DAI on Ethereum Mainnet and switches to Sepolia. Another @@ -489,7 +497,10 @@ test("a token another page added on Ethereum Mainnet stays when this page adds a const stored = boot.storage.read("autistmask"); expect(stored.networkTrackedTokens.mainnet).toEqual([MAINNET_TOKEN]); expect(stored.networkTrackedTokens.sepolia).toEqual([SEPOLIA_TOKEN]); - expect(stored.trackedTokens).toEqual([SEPOLIA_TOKEN, added]); + expect(stored.trackedTokens).toEqual([ + { ...SEPOLIA_TOKEN, balanceNotFetched: true }, + added, + ]); }); // This page tracks DAI on Ethereum Mainnet and switches to Sepolia, where TST is @@ -513,7 +524,10 @@ test("a token another page removed on Ethereum Mainnet stays removed when this p const stored = boot.storage.read("autistmask"); expect(stored.networkTrackedTokens.mainnet).toEqual([]); expect(stored.networkTrackedTokens.sepolia).toEqual([SEPOLIA_TOKEN]); - expect(stored.trackedTokens).toEqual([SEPOLIA_TOKEN, added]); + expect(stored.trackedTokens).toEqual([ + { ...SEPOLIA_TOKEN, balanceNotFetched: true }, + added, + ]); }); // Another popup page records a fraud contract on Ethereum Mainnet after this @@ -1031,7 +1045,9 @@ describe("a contract lookup that answers after a switch adds nothing", () => { await addAcrossSwitch(boot, "btn-settings-addtoken-manual", "mainnet"); expect(boot.visibleViews()).toEqual(["settings"]); expect(boot.text("flash-msg")).toBe(NETWORK_CHANGED_FLASH); - expect(stored().trackedTokens).toEqual([MAINNET_TOKEN]); + expect(stored().trackedTokens).toEqual([ + { ...MAINNET_TOKEN, balanceNotFetched: true }, + ]); expect(stored().networkTrackedTokens.sepolia).toEqual([]); }); @@ -1044,7 +1060,9 @@ describe("a contract lookup that answers after a switch adds nothing", () => { await addAcrossSwitch(boot, "btn-add-token-confirm", "sepolia"); expect(boot.visibleViews()).toEqual(["settings"]); expect(boot.text("flash-msg")).toBe(NETWORK_CHANGED_FLASH); - expect(stored().trackedTokens).toEqual([SEPOLIA_TOKEN]); + expect(stored().trackedTokens).toEqual([ + { ...SEPOLIA_TOKEN, balanceNotFetched: true }, + ]); expect(stored().networkTrackedTokens.mainnet).toEqual([MAINNET_TOKEN]); }); });