From 3e9e3300576e4659367df5836f14835257c96b70 Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 27 Oct 2026 17:31:18 +0000 Subject: [PATCH] fix: a refresh asked for while another runs is no longer dropped (closes #581) doRefreshAndRender() returned at once when a refresh was already running. An import asks for a refresh after its address scan, so the addresses the scan found showed 0.0000 until the next ten-second refresh. A request made while a refresh runs now sets a flag. One more refresh runs once the running one ends. Several requests in that time lead to one more refresh, not several. A tick of the ten-second timer during a refresh counts as such a request too. Model: opus-5-5 --- src/popup/index.js | 26 ++++-- tests/refreshDuringRefresh.test.js | 133 +++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 5 deletions(-) create mode 100644 tests/refreshDuringRefresh.test.js diff --git a/src/popup/index.js b/src/popup/index.js index f7cc77f..ec99dd2 100644 --- a/src/popup/index.js +++ b/src/popup/index.js @@ -65,7 +65,14 @@ function renderWalletList() { const pageClosed = new AbortController(); window.addEventListener("pagehide", () => pageClosed.abort()); +// A refresh asked for while another runs is not dropped. The request sets +// refreshAgain, and one more refresh runs once the running one ends. Several +// requests in that time lead to one more refresh, not several. An import asks +// for a refresh after its address scan. A refresh already running does not +// have the addresses the scan found +// (https://git.eeqj.de/sneak/AutistMask/issues/581). let refreshInFlight = false; +let refreshAgain = false; // The ten-second refresh startRefresh() starts, stopped when the popup moves to // the recovery screen: there is no profile left to refresh. @@ -89,7 +96,10 @@ let refreshTimer = null; // user turned "Use Blockscout" off. So the save drops every token the user has // not tracked if the stored setting is off by then. async function doRefreshAndRender() { - if (refreshInFlight) return; + if (refreshInFlight) { + refreshAgain = true; + return; + } refreshInFlight = true; try { await followStoredSettings(); @@ -133,8 +143,9 @@ async function doRefreshAndRender() { renderWalletList(); } 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 - // reject: an unhandled rejection is not a report of anything. The save + // second interval, eight views through ctx, and the end of this + // function for one more refresh — so it must never reject: an + // unhandled rejection is not a report of anything. The save // inside it reports its own failure through onSaveFailure() (see // src/shared/state.js); what is left here is a failed network round // trip, which the next tick retries. @@ -142,13 +153,18 @@ async function doRefreshAndRender() { } finally { refreshInFlight = false; } + if (refreshAgain) { + refreshAgain = false; + doRefreshAndRender(); + } } // Refreshes now and every ten seconds from then on. Called once a wallet // exists: by init() when the popup opens with one, and by AddWallet when it // adds one, which in a popup that opened on Welcome is the first -// (https://git.eeqj.de/sneak/AutistMask/issues/530). With the refresh already -// running it starts no second one, and on the recovery screen it does nothing. +// (https://git.eeqj.de/sneak/AutistMask/issues/530). With the ten-second +// refresh already started, it starts no second one. On the recovery screen it +// does nothing. function startRefresh() { if (!$("view-state-recovery").classList.contains("hidden")) return; doRefreshAndRender(); diff --git a/tests/refreshDuringRefresh.test.js b/tests/refreshDuringRefresh.test.js new file mode 100644 index 0000000..c365f40 --- /dev/null +++ b/tests/refreshDuringRefresh.test.js @@ -0,0 +1,133 @@ +// A refresh asked for while another runs is not dropped. One more refresh runs +// once the running one ends, however many were asked for in that time +// (https://git.eeqj.de/sneak/AutistMask/issues/581). Each test boots the real +// popup over a stored profile and holds the ten-second refresh open. The +// balance stand-in writes what the chain holds onto every address, as the real +// refreshBalances() does. + +const { + bootPopup, + cleanupPopup, + unversionedValidProfile, + ADDRESS, +} = require("./support/popupBoot"); +const { + hdWalletFromMnemonic, + deriveAddressFromXpub, +} = require("../src/shared/wallet"); + +const PHRASE = + "abandon abandon abandon abandon abandon abandon " + + "abandon abandon abandon abandon abandon about"; +const PASSWORD = "correct horse battery staple"; + +// The first two addresses of PHRASE, both used on the chain. +const { xpub, firstAddress } = hdWalletFromMnemonic(PHRASE); +const SECOND_ADDRESS = deriveAddressFromXpub(xpub, 1); + +// The ETH balance the chain holds for each address, in ether. +const CHAIN = { + [ADDRESS]: "1.5", + [firstAddress]: "2.0", + [SECOND_ADDRESS]: "3.0", +}; + +afterEach(() => { + cleanupPopup(); +}); + +// The balance stand-in for a refresh that returns. +async function readChain(wallets) { + for (const wallet of wallets) { + for (const addr of wallet.addresses) { + addr.balance = CHAIN[addr.address]; + } + } +} + +// Boots the popup over a profile with one wallet. From then on, a refresh the +// test does not answer never returns. So a refresh that starts another on its +// own is counted and stops, rather than running forever. +async function boot() { + const page = await bootPopup(unversionedValidProfile()); + expect(page.pageErrors).toEqual([]); + const { refreshBalances } = require("../src/shared/balances"); + refreshBalances.mockImplementation(() => new Promise(() => {})); + return { page, refreshBalances }; +} + +// Starts the ten-second refresh and holds it open until finish() is called. +// The held refresh then writes the chain's balances. finish() resolves once +// the held refresh has saved them and drawn Home. The one refresh after the +// held one also returns. +async function holdRefresh(page) { + const { refreshBalances } = require("../src/shared/balances"); + let answer; + refreshBalances + .mockImplementationOnce(async (wallets) => { + await new Promise((resolve) => { + answer = resolve; + }); + await readChain(wallets); + }) + .mockImplementationOnce(readChain); + const ticking = page.tick(); + await page.settle(); + return { + finish: async () => { + answer(); + await ticking; + }, + }; +} + +// Imports PHRASE on AddWallet. Its address scan finds the phrase's first two +// addresses. +async function importPhrase(page) { + const { scanForAddresses } = require("../src/shared/balances"); + scanForAddresses.mockImplementationOnce(async () => ({ + addresses: [ + { address: firstAddress, index: 0 }, + { address: SECOND_ADDRESS, index: 1 }, + ], + nextIndex: 2, + })); + await page.click("btn-add-wallet-bottom"); + await page.click("tab-mnemonic"); + page.node("wallet-mnemonic").value = PHRASE; + page.node("add-wallet-password").value = PASSWORD; + page.node("add-wallet-password-confirm").value = PASSWORD; + await page.click("btn-add-wallet-confirm"); +} + +test("addresses an import finds while a refresh runs show their balances once one more refresh returns", async () => { + const { page } = await boot(); + const refresh = await holdRefresh(page); + + await importPhrase(page); + expect(page.visibleViews()).toEqual(["main"]); + expect(page.text("flash-msg")).toBe("Found 2 addresses."); + + await refresh.finish(); + await page.settle(); + + const list = page.node("wallet-list").innerHTML; + expect(list).toContain("2.0000"); + expect(list).toContain("3.0000"); +}); + +test("several refreshes asked for while one runs lead to exactly one more", async () => { + const { page, refreshBalances } = await boot(); + const refresh = await holdRefresh(page); + const calls = refreshBalances.mock.calls.length; + + await page.tick(); + await page.tick(); + await importPhrase(page); + expect(refreshBalances).toHaveBeenCalledTimes(calls); + + await refresh.finish(); + await page.settle(); + + expect(refreshBalances).toHaveBeenCalledTimes(calls + 1); +});